Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Refactor smithy_types::Error to be more flexible (Breaking Change) #426

Merged
merged 4 commits into from
May 27, 2021

Conversation

rcoh
Copy link
Collaborator

@rcoh rcoh commented May 27, 2021

Issue #, if available: #422

Description of changes:
S3 (and probably other) services have errors with an additional set of metadata that must be recorded. This diff does a few things:

  1. Make the internal fields of smithy_error::Error private (as they should have been from the beginning)
  2. Adds a builder to easily construct errors
  3. Adds a property bag to enable storing additional fields on the error.

This will make it straightforward to add per-service customizations to record additional error metadata. Services will define their own extension traits to add strongly-typed property bag accessors.

Breaking Change: This is a breaking change because fields on error that were public are now private.
Fix: Change code that accessed fields directly to use accessors:

// before:
err.message

// after:
err.message()

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

S3 (and probably other) services have errors with an additional set of metadata that must be recorded. This diff does a few things:
1. Make the internal fields of smithy_error::Error private (as they should have been from the beginning)
2. Adds a builder to easily construct errors
3. Adds a property bag to enable storing additional fields on the error.

This will make it straightforward to add per-service customizations to record additional error metadata.
@rcoh rcoh requested a review from a team May 27, 2021 14:26
Copy link
Collaborator

@jdisanti jdisanti left a comment

Choose a reason for hiding this comment

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

LGTM!

@rcoh rcoh merged commit 43936cb into main May 27, 2021
@rcoh rcoh deleted the error-refactor branch May 27, 2021 16:28
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.

2 participants