Giter Site home page Giter Site logo

Improve Http Error messages about middy HOT 15 CLOSED

middyjs avatar middyjs commented on April 28, 2024 14
Improve Http Error messages

from middy.

Comments (15)

dbartholomae avatar dbartholomae commented on April 28, 2024 3

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

from middy.

jacintoArias avatar jacintoArias commented on April 28, 2024 2

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

from middy.

jacintoArias avatar jacintoArias commented on April 28, 2024 2

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 ;)

from middy.

theburningmonk avatar theburningmonk commented on April 28, 2024 1

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)

from middy.

willfarrell avatar willfarrell commented on April 28, 2024 1

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

from middy.

o-alexandrov avatar o-alexandrov commented on April 28, 2024

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.

from middy.

DavidWells avatar DavidWells commented on April 28, 2024

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

from middy.

lmammino avatar lmammino commented on April 28, 2024

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 😉

from middy.

DavidWells avatar DavidWells commented on April 28, 2024

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 =)

from middy.

theburningmonk avatar theburningmonk commented on April 28, 2024

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!

from middy.

lmammino avatar lmammino commented on April 28, 2024

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?

from middy.

lmammino avatar lmammino commented on April 28, 2024

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

from middy.

lmammino avatar lmammino commented on April 28, 2024

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.

from middy.

dbartholomae avatar dbartholomae commented on April 28, 2024

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.

from middy.

GreGosPhaTos avatar GreGosPhaTos commented on April 28, 2024

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.

from middy.

Related Issues (20)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.