-
Notifications
You must be signed in to change notification settings - Fork 8
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
Better alignment of errors in Polymorph with Smithy semantics #450
Comments
|
Good call, "best effort" is the wrong phrase here. Replaced with: "We only ensure that the error is retained and possible to log or otherwise extract programmatically somehow, just not in a clean and consistent way across programming languages." Edit: My edit didn't stick for some reason, re-applied |
Note that I'm not planning on acting on this design for a while - the models of existing smithy-dafny projects like https://github.com/aws/aws-cryptographic-material-providers-library are designed with the assumption that features like |
This MAY be better expressed as separate Issue, I would like to say "Unexpected error encountered while executing a DDB BatchGetItem", |
|
So that we can talk about it and stuff here is the simplest way that I see you could translate the above model into something like Dafny. There are a few issues, but I leave this as a starting point for us all to work from. module FooExample {
datatype BadThingError = BadThingError(
nameonly message: string
)
datatype SomeOtherError = SomeOtherError(
nameonly message: string
)
datatype FooError =
| BadThingError (
nameonly BadThingError: BadThingError
)
| SomeOtherError(
nameonly SomeOtherError: SomeOtherError
)
type FooErrorList = seq<FooError>
datatype FooErrors = FooErrors(
nameonly errors: FooErrorList
)
datatype FooOperationErrors =
| FooErrors(FooErrors: FooErrors)
method Foo ( input: FooInput )
returns (output: Result<FooOutput, FooOperationErrors>)
datatype Error = Error
datatype Result<T,E> = Result
datatype Option<T> = Option
datatype FooInput = FooInput
datatype FooOutput = FooOutput
} Updated to deal with |
So here is a straw dog proposal. module FooExample {
import AwsCryptographyKeyStoreTypes
datatype BadThingError = BadThingError(
nameonly message: string,
// this deals with the case where we have some underlying issue,
// but this issue is not the thing we want to return.
// This is how we can let a KMS error get mapped into a Keyring failure.
nameonly causedBy: Option<Error> := None
)
datatype SomeOtherError = SomeOtherError(
nameonly message: string,
nameonly causedBy: Option<Error> := None
)
datatype FooError =
| BadThingError(BadThingError: BadThingError)
| SomeOtherError(SomeOtherError: SomeOtherError)
type FooErrorList = seq<FooError>
datatype FooErrors = FooErrors(
nameonly errors: FooErrorList
)
datatype FooOperationErrors =
| FooErrors(FooErrors: FooErrors)
| Opaque(obj: object)
method Foo ( input: FooInput )
returns (output: Result<FooOutput, FooOperationErrors>)
// This operation is annotated with `@OldBehaviorBikeShedName`
method Bar ( input: BarInput )
returns (output: Result<BarOutput, Error>)
datatype Error =
| BadThingError(
nameonly message: string,
nameonly causedBy: Option<Error> := None
)
| SomeOtherError(
nameonly message: string,
nameonly causedBy: Option<Error> := None
)
| FooErrors(FooErrors: FooErrors)
| AwsCryptographyKeyStore(AwsCryptographyKeyStore: AwsCryptographyKeyStoreTypes.Error)
| Opaque(obj: object)
datatype Result<T,E> = Result
datatype Option<T> = None
datatype FooInput = FooInput
datatype FooOutput = FooOutput
datatype BarInput = BarInput
datatype BarOutput = BarOutput
}
module AwsCryptographyKeyStoreTypes {
datatype Error = Error
} |
One of the reasons errors are modeled in Smithy is so that customer code in any programming language can handle at least some errors programmatically. This includes being able to test for a specific type of error, and possibly reading structured values out of it.
Many programming languages will use polymorphism to support error handling in general. In Java, for example, you can throw and catch any subtype of
Exception
, and test if the exception you’re holding onto is a specific kind with something likee instanceof ThrottlingException
. But Smithy does not support arbitrary polymorphism by design, and it is not idiomatic in all languages to define or use an open polymorphic error type.Dafny is similar to Rust in this way: it has no special built in error type, and no universal top type, so it isn’t possible to hold onto an arbitrary “error value”.
smithy-dafny
currently generates a single top-level service-specificError
type that can represent any error from the whole service, and any dependent service in the case of@localService
s. This includes anOpaqueError(error: object)
data constructor, which is perfectly reasonable as any client or service or polymorph library needs to be able to represent unmodeled errors somehow. But automatically making any error from any service in the whole dependency graph part of the API doesn’t align with Smithy semantics, and therefore complicates the Polymorph implementation for each supported language and hurts reuse of existingsmithy-<lang>
tools. It is especially a problem for@extendable
resource operations, because new implementations should not be able to add more error possibilities.Therefore I propose the following changes:
We clarify the semantics of Polymorph to align with the Smithy specification: that only the errors listed in an operation’s
errors
(or indirectly in the service’s commonerrors
list) can be handled programmatically as structured result values in each target language. We call any other kind of error “unmodeled errors”, even though the underlying error value may correspond to some other Smithy@error
shape.Unmodeled errors MAY have to be wrapped as an unhandled error of some kind, i.e. as a different type than how it would be represented as a modeled error for a given operation.
We only make a best effort to include as much detail as possible when the error is displayed/logged/etc.Edit: We only ensure that the error is retained and possible to log or otherwise extract programmatically somehow, just not in a clean and consistent way across programming languages.Error
type.The Dafny interface for services currently uses the single service-specific
Error
type everywhere, for both SDKs and local services. We will keep this type for use in implementations, but also generate per-operation error subset types and use them in the operation signatures instead, to more accurately represent Smithy semantics. That is, the signaturereturns (output: Result<FooOutput, Error>)
will becomereturns (output: Result<FooOutput, FooError>)
, whereFooError
is a subset type withError
as a base type, restricting to only the explicitly declared error shapes. This is thankfully not a breaking change since it's only making a return type more specific.The generated code connects the abstract
MyService.Foo
toMyServiceOperations.Foo
will call a generated method to convert aError
to aFooError
by converting all variants that aren’t declared in the operation’serrors
toOpaqueError
values instead.This will require an extern to wrap an arbitrary Dafny value as a Dafny
object
, which isn’t generally possible since not all Dafny values are reference types. If possible we should even modifyOpaqueError
to hold onto a completely opaque extern type declared in the smithy-dafny StandardLibrary instead ofobject
.We will also modify the conversion logic to wrap target language interfaces back into Dafny (“Wrapped services”) to wrap up unmodeled errors as
OpaqueError
. This is important to ensure wrapped service tests cannot require error handling behavior that cannot/should not actually be supported in a given language, such as testing forerror.DependencyA.DependencyB.SomeError?
(as we do here for example).Since existing polymorph library operations have been under specifying the set of errors, we can incrementally add the key error shapes that customers will commonly want to handle programmatically to their Smithy models.
Instead of requiring that the nested structure of
MyService.Error
is maintained in all languages, we must instead ensure that error information is not lost when working with unmodeled errors. We will replace tests like this with tests that unmodeled errors, especially those from target language implementations, are chained and wrapped as needed to ensure that logging them can include relevant information.On CollectionOfErrors
It should always be possible to enable programmatic handling of the set of
errors
for an operation somehow in a given programming language, because the result of aFoo
operation can always be represented as an equivalent Smithyunion
of either a successful output structure or one or more error variants:Therefore as long as a code generator supports unions, it can support errors, even if it has to use the same kind of datatypes to handle the requirements for unions. Rust for example, generates a
FooError
enum
that looks similar, plus anUnhandled
variant.CollectionOfErrors
is absolutely useful for Dafny-implemented polymorph libraries, for the same reason that features like exception chaining are useful in other languages: it lets you hang onto the details of more than one error in a single value. However,CollectionOfErrors
is particularly difficult to reconcile with the Smithy design: the Dafny version of it is essentially a list ofMyService.Error
values, meaning each element can be any error forMyService
, andMyService.Error
has no corresponding shape in the original Smithy model.Therefore we should consider the Dafny
CollectionOfErrors
variant an unmodeled error, which we only ensure can be printed with all details and not necessarily programmatically handled. In cases where customers may want to programmatically handle a collection of errors, this can be modeled explicitly in the Smithy operation:This is fairly similar to how AWS services model the results of batch operations. Note that the structure of the Dafny
FooError
generated for this model ends up looking very similar to the existingCollectionOfErrors
variant thatsmithy-dafny
always generates.The text was updated successfully, but these errors were encountered: