Middy: @middy/http-error-handler to respect the `expose` property?

[raae]

(Thanks for reporting an issue! Please make sure you click the link above to view the issue guidelines, then fill out the blanks below.)

NOTE: There seems to be no link above...

---

I think the @middy/http-error-handler should respect the expose property of http-errors.
I am happy to fix this if wanted.

What are the steps to reproduce this issue?

1. Use the @middy/http-error-handler middleware
2. Throw a httpError with expose=false in middleware or handler

What happens?

The response includes the error message

What were you expecting to happen?

The response to not include the error message

Any other comments?

From the http-errors docs:

expose - can be used to signal if message should be sent to the client, defaulting to false when status >= 500

What versions of software are you using?

Not relevant

[willfarrell]

I think this is a great idea. If you can put a PR together, that would be awesome. I can port it to release/v2.x, or we can have this included in v2.x only. I'll leave it up to you.

[AyoAlfonso]

If I were to jump on this @willfarrell is this where I should be looking [https://github.com/middyjs/middy/tree/master/packages/http-error-handler] ?

[willfarrell]

v1: https://github.com/middyjs/middy/tree/master/packages/http-error-handler
v2: https://github.com/middyjs/middy/blob/release/2.x/packages/http-error-handler

[AyoAlfonso]

What I am pushing will look like this;

1. Get the expose prop
2. Run logger if except value is true AND statusCode is NOT 500 ( Dependency http-errors uses standard error codes from native Node module)
3. Make the default options.logger function handle the last possible case

[willfarrell]

I've pushed a change to the v2 branch w/ tests. Let me know if this is what you had in mind: https://github.com/middyjs/middy/blob/release/2.x/packages/http-error-handler/index.js

[AyoAlfonso]

Yes, what I did is something like this but on version one.

Something I want to point out is this line, shouldn't the logic here be < 500?
expose should be true for anything less than a 500 error code

I believe that should mean

if (handler.error?.statusCode && handler.error?.expose === undefined) {
      handler.error.expose = handler.error.statusCode < 500
    }

instead of

if (handler.error?.statusCode && handler.error?.expose === undefined) {
      handler.error.expose = handler.error.statusCode >= 500
    }

or am I missing something

[raae]

There is also the possibility to check if it is a httpError, instead of checking for expose:

var createError = require('http-errors')
var error = createError(404, err, { expose: false })
createError.isHttpError(error) // true

[raae]

I my code I did something along the lines of

1. Check if it is an httpError,
2. if not make an httpError of the error info received
   
   
   
   • will get expose set to false for < 500 automatically
   
   
3. Then use the httpError and check for expose

Just some thoughts, no need to incorporate...

[AyoAlfonso]

I think there is a typing for that, you can check the accompanying index.d.ts file @raae 🙂

[AyoAlfonso]

  if (handler.error?.statusCode) {
      handler.error.expose =
        !handler.error?.expose
          ? false
          : handler.error.statusCode < 500
    }

I think something like this should pass.
Also it won't be a bad idea to incorporate @raae 's idea at very beginning of the httpErrorHandlerMiddlewareOnError function

[willfarrell]

@AyoAlfonso Good catch, I've added in more tests and fixed the logic.

@raae Not including http-error is by design, though I was tempted to last night. By not including it, the codebase size stays small and allows for custom errors that may not strictly follow all requirements of the http-error pattern.

I've also updated the docs to reflect the breaking change to errors with statusCode >= 500. Thanks for your help in adding this in.

Ref:

• createError.isHttpError: https://github.com/jshttp/http-errors/blob/6e4f655ec3a0cedf2e3ce868daa11b9210d1f103/index.js#L182