-
Notifications
You must be signed in to change notification settings - Fork 105
Add generated error types for convenience #298
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
Conversation
import software.amazon.smithy.model.shapes.StructureShape; | ||
import software.amazon.smithy.model.traits.ErrorTrait; | ||
|
||
final class ServerErrorGenerator implements Runnable { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should add a comment explaining why this is here, particularly when it's not being done for clients.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
okay, I added it
Symbol symbol = symbolProvider.toSymbol(errorShape); | ||
|
||
// Did not add this as a symbol to the error shape since these should not be used by anyone but the user. | ||
String typeName = symbol.getName() + "Error"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is suffixing errors being used to get around having to alias the error interface? It would probably be better to construct a SymbolReference
for the error like so:
SymbolReference symbol = SymbolReference.builder()
.symbol(errorInterfaceSymbol)
.alias("__" + errorInterfaceSymbol.getName())
.build();
Of course you could also do:
String errorInterfaceAlias = "__" + errorInterfaceSymbol.getName();
writer.addImport(errorInterfaceSymbol, errorInterfaceAlias, SymbolReference.ContextOption.USE);
And then manually pass around the name string, which is effectively what the SymbolReference
gives you.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is to work around the fact that we can't export the type from models
and then export a type of the same name here. Honestly, it works out; instead of throwing new ResourceNotFound()
, you throw new ResourceNotFoundError()
, and people will probably figure out that they don't want to postfix their error structure names with Error
.
It's distinctly unpleasant to throw a constructed object that has to include matching values of name, $fault, etc to an interface. This generates convenience classes for services that allow errors to be thrown in a more user-friendly fashion.
* Make the order for framework errors stable * Generate convenience types for operation errors It's distinctly unpleasant to throw a constructed object that has to include matching values of name, $fault, etc to an interface. This generates convenience classes for services that allow errors to be thrown in a more user-friendly fashion.
Description of changes:
It's really annoying to throw objects that have a bunch of fixed values. This turns:
into
By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.