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

Allow Error traits to be renamed, make original fully qualified name available to code generators #1545

Closed
everett1992 opened this issue Dec 15, 2022 · 4 comments
Labels
feature-request A feature should be added or improved.

Comments

@everett1992
Copy link

Shapes with an ErrotTrait cannot be renamed, attempting a rename throws an error like
https://github.com/awslabs/smithy/blob/bf9fc069338fe78ed3dfc1f51820a0101dfec12b/smithy-model/src/test/resources/software/amazon/smithy/model/errorfiles/validators/service-rename/error-rename-invalid.errors#L1

https://github.com/awslabs/smithy/blob/f598b87c51af5943686e38706847a5091fe718da/smithy-model/src/main/java/software/amazon/smithy/model/validation/validators/ServiceValidator.java#L159-L160

Presumably this is to support decoding the correct error type from protocols that only encode the shape name. However, there are other protocols that encode the fully qualified name in errors. These protocol can support renamed Errors if the fully qualified original name is preserved in the model so codegen can reference the original name.

@mtdowling
Copy link
Member

We're thinking that maybe we can make this limitation around not being able to rename errors protocol-specific rather than built-in to all services. That way, protocols that don't use namespaces on the wire can validate and forbid it, while others can support it.

@everett1992
Copy link
Author

everett1992 commented Dec 19, 2022

That makes sense.

If a protocol used namespaces over the wire would it use shapeId.getName() to get the original name, rather than shapeId.getName(service) to get the service renamed name?

https://github.com/awslabs/smithy/blob/main/smithy-model/src/main/java/software/amazon/smithy/model/shapes/ShapeId.java#L306-L335

@mtdowling
Copy link
Member

I would expect it to use shapeId.getName() since renaming shapes doesn't assign it a new shape ID, it just gives it a unique name within a service closure (primarily used for codegen).

@kstich
Copy link
Contributor

kstich commented Jul 19, 2023

This was resolved on the Smithy side in #1554

@kstich kstich closed this as completed Jul 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-request A feature should be added or improved.
Projects
None yet
Development

No branches or pull requests

4 participants