Skip to content

fix error serialization for default error members#6583

Merged
alexrashed merged 4 commits intomasterfrom
fix-error-shape-serialization
Aug 3, 2022
Merged

fix error serialization for default error members#6583
alexrashed merged 4 commits intomasterfrom
fix-error-shape-serialization

Conversation

@alexrashed
Copy link
Member

This PR addresses an issue with the error serialization mentioned in #6571.

After some digging, it turns out that there are several XML-services (rest-xml, query) which define default members in their error shapes (f.e. cloudfront, route53, cloudwatch, iam,...). These default members are not added to the root node, while non-default members are.

For JSON-services (json, rest-json), there is no sub-element for the error metadata. All members are added to the root node. Here, defined default members in the spec can modify the casing (f.e. Message in DynamoDB's TransactionCanceledException).

The following changes are contained in this PR:

  • Enhance the error serialization for error shapes which define additional members which "conflict" with the default error fields (like "message" or "code").
  • Remove the member "type" from the special handling (this was a wrong assumption in the past, it doesn't correlate with the code, only __type for json protocols do). F.e. in Lambda, the "Type" field on exceptions is "SOAP"-like and contains the value "User" to indicate that it's a user-triggered error.
  • Fix the scaffold to also generate an additional "Type" field (f.e. for Lambda).
  • Regenerate the APIs (since the scaffold was modified).
  • Clean up some tests and added some more.

@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 3, 2022 12:31 Inactive
Copy link
Member

@thrau thrau left a comment

Choose a reason for hiding this comment

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

LGTM! that's quite some digging you did there :-)

Copy link
Contributor

@bentsku bentsku left a comment

Choose a reason for hiding this comment

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

LGTM! Wow, thanks for digging into this so fast and finding a solution! 🙏

@github-actions
Copy link

github-actions bot commented Aug 3, 2022

LocalStack integration with Pro

       3 files  ±0         3 suites  ±0   1h 5m 32s ⏱️ - 15m 20s
1 160 tests +2  1 118 ✔️ +1  42 💤 +1  0 ±0 
1 521 runs  +6  1 448 ✔️ +3  73 💤 +3  0 ±0 

Results for commit 00e2193. ± Comparison against base commit c341336.

♻️ This comment has been updated with latest results.

@alexrashed alexrashed temporarily deployed to localstack-ext-tests August 3, 2022 13:39 Inactive
@coveralls
Copy link

Coverage Status

Coverage decreased (-0.002%) to 91.59% when pulling 00e2193 on fix-error-shape-serialization into c341336 on master.

@alexrashed alexrashed merged commit 30d2112 into master Aug 3, 2022
@alexrashed alexrashed deleted the fix-error-shape-serialization branch August 3, 2022 14:49
@localstack localstack locked and limited conversation to collaborators Aug 3, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants