aws-powertools/powertools-lambda-typescript

Maintenance: make error handlers' response more versatile

Closed this issue · 8 comments

dreamorosi commented

Summary

Currently error handlers must return an ErrorResponse object with type:

type ErrorResponse = {
  statusCode: HttpStatusCode;
  error: string;
  message: string;
};

While building with the new Event Handler I had a number of ideas that I'd like us to experiment:

  • Can we change the return type of error handlers to ErrorResponse | Response? This would help in case customers want to have full control over what's returned by an error handler, similar to what they can do in regular handlers
  • For named error handlers only (i.e. notFound and methodNotAllowed ) we should consider making the statusCode field from ErrorResponse optional, since these method already have a default one
  • Right now both error and message are required, can we update the type so that at least one can be omitted? this reduces repetition in case I care about only one
  • Finally, can I throw any of the built-in error classes (i.e. NotFoundError) from an error handler? Right now if I do that, the Event Handler interprets it as a runtime error and falls back to the default error handler (502)

Why is this needed?

None of the ideas proposed above is required per se, especially if it doesn't make sense or complicates the overall API, but I'd like us to consider them as I think they'll make the API more ergonomic.

Which area does this relate to?

Event Handler

Solution

No response

Acknowledgment

Future readers

Please react with 👍 and your use case to help us understand customer demand.

dreamorosi commented

@sdangol assigning this to you - when implementing, please let me know if you find any blocker related to some of the questions in the issue. If you do, ping me and we'll decide what to do before going too deep with the reactor.

  • 👍1
sdangol commented

Can we change the return type of error handlers to ErrorResponse | Response? This would help in case customers want to have full control over what's returned by an error handler, similar to what they can do in regular handlers

@dreamorosi @svozza , for this, should we expect the customers to create the response object themselves as

const app = new Router();

app.errorHandler(BadRequestError, async () => {
        return new Response(JSON.stringify({message: "There was an error processing your request"}), {
          status: HttpErrorCodes.BAD_REQUEST,
          headers: { 'Content-Type': 'application/json' },
        });
});

or should we support it even without creating the response object. Something like

const app = new Router();

app.errorHandler(BadRequestError, async () => {
        return {
            statusCode: HttpErrorCodes.BAD_REQUEST,
            message: "There was an error processing your request",
            foo: 'bar'
        }
});
svozza commented

We want to support both, which is how the standard handlers work.

  • 👍1
sdangol commented

We want to support both, which is how the standard handlers work.

If we were to support the ErrorHandler return type to be ErrorResponse | Response | JSONObject, what should we use for the statusCode when a JSONObject is used?

dreamorosi commented

Good question - I think we should have a default, perhaps a generic ServiceError 5xx code and allow customers to override it in the response.

Thoughts?

svozza commented

Yeah, I think 500 is the most reasonable one to fall back to.

  • 👍1
powertools-for-aws-oss-automation commented

Warning

This issue is now closed. Please be mindful that future comments are hard for our team to see.
If you need more assistance, please either reopen the issue, or open a new issue referencing this one.
If you wish to keep having a conversation with other community members under this issue feel free to do so.

github-actions commented

This is now released under v2.27.0 version!