Skip to content

fix(rest): making non 200 rest errors more informative#972

Merged
laskoviymishka merged 3 commits into
apache:mainfrom
happydave1:fix/non200err
May 4, 2026
Merged

fix(rest): making non 200 rest errors more informative#972
laskoviymishka merged 3 commits into
apache:mainfrom
happydave1:fix/non200err

Conversation

@happydave1
Copy link
Copy Markdown
Contributor

fixes #971

Signed-off-by: happydave1 <dzhao2004@gmail.com>
@happydave1 happydave1 requested a review from zeroshade as a code owner May 2, 2026 21:14
Copy link
Copy Markdown
Contributor

@laskoviymishka laskoviymishka left a comment

Choose a reason for hiding this comment

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

Direction looks right, and handling the flat S3 Tables {"message":"..."} body makes sense.

I’d request changes before merging though, mainly because this is an error-decoding edge case and currently has no tests. At minimum, please add coverage for:

  • flat {"message":"x"} preserving the status sentinel via errors.Is
  • canonical {"error":{"message":"x","type":"y"}} still rendering as expected
  • empty body / HEAD falling back to the wrapped status error instead of ": "
  • the new Error() formatting branches

One small code concern too: the new Code field is decoded but never used. Either include it in the rendered error, or drop it.

Also, e = *payload.Error feels a bit fragile. It would be safer to keep decoding into &e as before and only fill the flat fields when there is no useful nested error.

@happydave1 happydave1 changed the title feat(rest): making non 200 rest errors more informative fix(rest): making non 200 rest errors more informative May 3, 2026
Signed-off-by: happydave1 <dzhao2004@gmail.com>
@happydave1 happydave1 requested a review from laskoviymishka May 4, 2026 06:27
Signed-off-by: happydave1 <dzhao2004@gmail.com>
Copy link
Copy Markdown
Contributor

@laskoviymishka laskoviymishka left a comment

Choose a reason for hiding this comment

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

Nice!

@laskoviymishka laskoviymishka merged commit 2b10494 into apache:main May 4, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Incompatible s3 table bucket error response - results in "blank" error

2 participants