Improve Http Error messages

[DavidWells]

I altered the httpErrorHandler to follow the JSON spec http://jsonapi.org/examples/#error-objects-basics.

Should this change get applied across the board? Or should I just use this as my own custom middleware?

Whats the default lambda integration you guys are using? (we typically use lambda-proxy as the serverless framework default)

Here is my altered error handler for nice errors =)

const { HttpError } = require('http-errors')

module.exports = () => ({
  onError: (handler, next) => {
    if (handler.error instanceof HttpError) {
        // as per JSON spec http://jsonapi.org/examples/#error-objects-basics
        handler.response = {
          body: JSON.stringify({
            errors: [{
              status: handler.error.statusCode,
              message: handler.error.message,
              detail: handler.error.details
            }]
        })
      }
      return next()
    }

    return next(handler.error)
  }
})

It looks like:

Before with body: handler.error.message (no json response)

Love this freakin project! Great work 🎉

[o-alexandrov]

For the unexpected 'E', don't you minify the code?
Isn't that the reason why the variable got untrackable?

Great to see an improvement in error tracking though.

[DavidWells]

The error is from the response returning a string instead of JSON via the lambda-proxy integration of API gateway

[lmammino]

Hello (again 😊) @DavidWells!

Thanks for the interesting input. Honestly it's my first time seeing the JSON API standard. It sounds pretty interesting but i would like to understand more all the implications before deciding whether it makes sense to adopt it by default in Middy or not.

Regarding our API gateway integration we prefer to use the Lambda proxy as well, as it makes easy to create standard middlewares and integrates well will the serverless framework.

Also, regardless the JSON API spec, error reporting and response serialization are a bit of open discussion at the moment. I would love to find an easy way to keep them separated for higher flexibility (imagine an API that can support both JSON and XML based on the client accept header). So, if you have any thought or ideas on this i'd love any input 😉

[DavidWells]

So maybe checking if 'Content-Type' is application/json like https://github.com/middyjs/middy/blob/master/src/middlewares/jsonBodyParser.js#L5 would work.

If json return the json error.

I'm open to open shapes of errors as well. There wasn't much concensus on how json apis should return errors in a standard way https://stackoverflow.com/questions/12806386/standard-json-api-response-format

Maybe @theburningmonk has some insight here?

I'm keen on making some tracing and logging middleware for middy =)

[theburningmonk]

Hi @DavidWells thanks for the nudge!

My personal feeling is that, as a default behaviour, we shouldn't readily return so much error details in the response, they often leak implementation details unwillingly, I have even seen errors that returns the IP address for redis/mongo servers (which wasn't secured... 😞).

It's great for debugging our endpoints, for us and for attackers probing our system.

Usually I approach error handling in roughly like this:

• custom error types for application specific errors, each with error code that falls within ranges - eg. 10000-10100 => validation error, 10101-10200 => error on some dependency, etc.
• report unique request ID (and maybe an error ID, which is usually short, and can be easily read out by someone over the phone, for example) in the HTTP response so they can be referenced in the logs, eg.

{
  "errorCode": 10101,
  "message": "[a';DROP TABLE users; SELECT * FROM userinfo WHERE 't' = 't] is not a valid first name!",
  "requestId": "some-long-guid",
  "errorId": "6-8 chars ID"
}

• in the middleware layer, the equivalent to onError would log the entire invocation event as ERROR, with accompanying correlation IDs including the current request ID, and originating request ID, etc.
• in the client, write logic to handle errors different by error code, and in cases where we can't gracefully recover, then show dialog to user that tells them to contact support with the requestId or errorId depending on the context (if it's a web app where you can automate the reporting, or easily copy and paste then requestId is fine, but on mobile you need to keep this user-facing ID really short), eg. oops, sorry that didn't work... please contact us at xxxx and quote the ID [6-8 chars] and we'll look into it for you!

[theburningmonk]

As for handling different content types, keep in mind that we might want to consider binary formats as well. What if you always inspect the content-type in the response, and defaults to JSON if it's not specified, and bundle only JSON handling in middy by default.

You can then later introduce XML, proto-buf and other formats as middleware? (in the case of protobuf, you also have to set the isBase64Encoded to true, for instance)

[lmammino]

Great to see this discussion happening :)

Thanks @DavidWells for having started it and to @OlzhasAlexandrov and @theburningmonk for the valuable contribution.

My current feeling is not to use the JSON API spec by default (even though it might make sense to have some level of support with dedicated middlewares).

Also, I would love to be able to have a generic concept of output serialization (which should work also for errors of course).

In code I would imagine this like the following:

const handler = middy((event, context, cb) => {})

handler
  .use(jsonSerializer())
  .use(jsonAPISerializer())
  .use(xmlSerializer())
  .use(protoBufferSerializer())

Every one of those serializer middlewares should check if the accept header of the request specifies

Or probably it would be easier to implement something like this:

const handler = middy((event, context, cb) => {})

handler
  .use(outputSerializer(
    [
       jsonAPISerializer(),
       xmlSerializer(),
       protoBufferSerializer()
    ]
   ))

This is something supported in many other web frameworks, so maybe we should take inspiration from the APIs of a famous one (i am of course open to suggestions).

Thoughts?

[lmammino]

Hey @DavidWells, based also on #94, I was trying to understand better this error case and see if it can be addressed somehow, but I wasn't able to fully reproduce the issue.

Can you please post your full code snippet to help me reproduce this case?

Thanks

[jacintoArias]

I came here after dealing myself with the same error pointed out by @DavidWells when implementing my custom error handling middleware. I ended up almost copying his solution to correctly follow the JSON API spec.

At least for the lambda proxy integration, the error response should change the body format or the content header, otherwise the response is not valid...

PS: Awesome work guys! I LOVE this project

[lmammino]

Hello @jacintoArias and thanks a million for your feedback.

Recently we added the http content negotiation middleware to middy. Based on that I am (very very unfortunately) working on a response serializer that should help with having a more cohesive and flexible experience on how we manage responses (including error ones).

Regarding the JSON API spec, if you feel as well it's something very common, I would be more for creating a dedicated middleware (e.g. httpJsonApiErrorHandler ) rather than making the current one, which is supposed to be small and generic, more specific.

[jacintoArias]

In the end is just a matter of specific use cases vs common patterns, guess is better to wait for the community to grow (I am sure it will grow!). Right now I have "middified almost everything in my lambdas".

Following @theburningmonk advices above I have created a set of custom errors for my libraries (data layer, services), that break the execution chain and are handled by a set of error middlewares.

I love the idea of using httpErrors to manage "handled" exceptions, so I'm catching the incoming custom errors and transforming them into common sense httpErrors in a specific middleware, I keep the traces and messages in "development mode" or just standard messages in production to hide the implementation as suggested.

middy(logic)
  (...) // other middleware
  .use(responseSerializer) // ::after - Yes, I have an approach for that too
  .use(customErrorHandler) // ::onError - Parses custom error types and transform them into httpErrors
  .use(httpJsonApiErrorHandler)  // ::onError - Adapt any httpError to JsonAPI, anything else into 500 generic error

I would be nice to have access to other patterns and use cases from other users (perhaps a gitter channel or is too early?). I would watch the repo and see if I can PR something ;)

[dbartholomae]

Just stumbled upon a similar problem because at least with serverless-offline there is a Content-type: application/json set, which is wrong for errors created with the current middleware. Any news on this discussion? Is there a middleware for JSON error messages already? Otherwise I might give it a try after finishing the JWT Auth middleware.

[willfarrell]

@DavidWells @jacintoArias Have you tried https://github.com/willfarrell/middy-jsonapi, I've been using it for about 2y now without issue.

[GreGosPhaTos]

Hi guys, just followed the discussion here, and the @willfarrell middleware seems to be the right way of implementing JSON api specs in your lambdas.
As mentioned above, in the httpErrorHandler I'm not sure that is good approach to implement any JSON responses, it is custom to your api and the way you want to manage the error.
In that case, better to implement your own error handler.
I would suggest to close this ? Otherwise, if you have any other suggestions, I would be happy to help.

[dbartholomae]

BTW here's the middleware I ended up writing:
https://github.com/dbartholomae/middy-middleware-json-error-handler