Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Fix "missing error information" error on 409s #4530

Merged
merged 1 commit into from
Jan 17, 2025

Conversation

matthchr
Copy link
Member

This can happen if the Azure RP being called returns a JSON error response payload that doesn't match the Azure contract. Instead of overwriting the underlying error, just return it as-is to aid in debugging.

  • this PR contains documentation
  • this PR contains tests
  • this PR contains YAML Samples

This can happen if the Azure RP being called returns a JSON error
response payload that doesn't match the Azure contract. Instead of
overwriting the underlying error, just return it as-is to aid in
debugging.
@matthchr
Copy link
Member Author

/ok-to-test sha=ca6ddee

Comment on lines +72 to +75
// error shape contract. The good news is that if this happens we know this isn't
// an unregistered RP because that API will have the right shape. We just return
// the resp/err directly in this case to let the caller deal with whatever the problem
// was as we know it wasn't unregistered RP.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't understand this - it appears to be saying that we know this is a registered RP because those have the right shape, but isn't the problem here due to the result having the wrong shape?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What I'm trying to say is, the error for unregistered RP will have the correct shape. So if we end up in this if (incorrect shape), it had to come from the RP, which means its registered, which means we can just return the error as-is we don't need to try to parse/process it.

Copy link
Member

@theunrepentantgeek theunrepentantgeek left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think one comment needs cleanup, but otherwise this looks good.

@matthchr matthchr added this pull request to the merge queue Jan 17, 2025
github-merge-queue bot pushed a commit that referenced this pull request Jan 17, 2025
This can happen if the Azure RP being called returns a JSON error
response payload that doesn't match the Azure contract. Instead of
overwriting the underlying error, just return it as-is to aid in
debugging.
@github-merge-queue github-merge-queue bot removed this pull request from the merge queue due to failed status checks Jan 17, 2025
@matthchr matthchr added this pull request to the merge queue Jan 17, 2025
Merged via the queue into Azure:main with commit f42e5bb Jan 17, 2025
7 checks passed
@matthchr matthchr deleted the feature/fix-malformed-error-handling branch January 17, 2025 23:56
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

3 participants