search

smithy-lang / smithy

Smithy is a protocol-agnostic interface definition language and set of tools for generating clients, servers, and documentation for any programming language.
https://smithy.io
Apache License 2.0
1.71k stars 201 forks source link

Provide Smithy models for default API Gateway errors #1170

Open eduardomourar opened 2 years ago

eduardomourar commented 2 years ago

Whenever we deploy our services using API Gateway from AWS, there are errors that we have no control over (as described here). The details of those errors are needed in order to allow the proper deserialization within the client SDKs. We have tried to solve this with two different approaches, although without success:

  1. By defining the model for them ourselves, but we have not been able to find what are the possible X-Amzn-ErrorType HTTP headers that are being sent by API Gateway. The closest possibility can apparently be found here.
  2. By overriding the X-Amzn-ErrorType HTTP header using the OpenAPI extension x-amazon-apigateway-gateway-responses. That caused the header to be sent two times containing our override value BadRequestException and the default value IncompleteSignatureException from API Gateway (as shown in the attached image). The client SDK does not seem to like that, and simply ignore our value. Screenshot 2022-04-03 at 19 52 12

Is is possible for AWS to provide the Smithy models only of those default API Gateway errors (similar to this here), please?

adamthom-amzn commented 2 years ago

The 5xx errors shouldn't need a model - the client should transform them to some sort of generic client exception.

The 4xx errors, particularly around input validation, we should look at a better experience for generating response templates for. The optimal experience for some of them, like bad request body or parameters, would be to remap them to a validation exception type (such as the built-in one). The others we might need to add an additional model for.

DanielBauman88 commented 1 year ago

What about making the generated client look at values in the X-Amzn-ErrorType header to match one of the errors defined on the operation?

If there's only one matching value - deserialize correctly to that exception. This is reasonable and not surprising behaviour and it unblocks customers who are using api gateway and gateway responses to have a functional sdk for these errors.

If there's no match or if there's ambiguity (values for two different defined errors) then continue with the existing behaviour.

The others we might need to add an additional model for.

Could you elaborate on this? Does this imply that smithy itself will define some error for these cases which all customers must use for these cases? Wouldn't it make sense for smithy to let customers define the errors and headers they want for their exceptions (including auth/throttling) without smithy making this choice for them?

DanielBauman88 commented 1 year ago

@adamthom-amzn what do you think of the two options in the PRs? Do you think one of them is viable for merging?

I think that customizing the header name is a safe and non-invasive change which enables customers to configure their APIs to their needs. It may not be strictly needed in the sdk package, but maybe it could live there until all the useful common logic that lives in the sdk is extracted and smithy clients are more usable without this dep.

gosar commented 1 year ago

@eduardomourar @DanielBauman88 are you unblocked with the change to js repo?

On the Smithy side, we'd still want to add a protocol test for this, so other language SDKs can also support this. So leaving this issue open for that followup.

DanielBauman88 commented 1 year ago

This does unblock me.

Appreciate you keeping this open - would be great to have the same support for all the supported languages!