httpError(N, error) is misleading if error is already an HTTP error

[innermatrix]

I would expect

httpError(500, httpError(402)).statusCode

to be 500, but it's 402.

(The use case here is that I have a try/catch statement that turns all exceptions into HTTP 500s by default, and it's not working correctly when the original exception is already an httpError.)

[dougwilson]

Mmm, yes, that is good point. Unfortunately it is the current behavior relied upon by existing uses (that a given error object will have it's status code preserved over the argument status code). This is regardless of the error being httpError or not. So the docs are not clear on this, but I also lile the idea of how you would like it to behave too, so I think we should support both somehow. I will think on the API for that 👍

[jonchurch]

Currently createErrors has a very loose args interface.

I was surprised this issue was possible, because we will ignore any status passed in via the additional props bag. But because we are passing something which is instanceof Error, we will honor the status.

it doesn't matter what position we pass an error in, if it is detected in args we will set the status there.

So consider also:

const createErrors = require('http-errors')
const err = new Error()
err.statusCode = 411

const createdError = createErrors(
 createErrors(404),
 createErrors(414),
 { bag: 'foo', status: 500 },
 createErrors(400, createErrors(err)),
 { bag: 'foo', status: 429 },
 404
)

console.log(createdError.statusCode === 411) // true

The last instance of an Error wins. The properties bag uses the props but ignores status

[AIEINC]

Well done

[mahmoodhamdi]

I ran into this same behavior in my Express error handling middleware. I had a catch-all that wraps unexpected errors:

app.use((err, req, res, next) => {
 const httpErr = createError(500, err)
 // Expected: always 500
 // Actual: if err was already an http-error with status 404, it stays 404
})

Looking at the code, the issue is in the argument parsing — when an Error is passed, its .status or .statusCode is extracted and overwrites the status variable:

if (type === 'object' && arg instanceof Error) {
 err = arg
 status = err.status || err.statusCode || status // <-- err's status wins
}

Then later, the explicit status argument is never re-applied because the error's own status has already taken its place.

I think the cleanest solution that wouldn't break existing users would be to give the explicit status code argument priority over the error's existing status. Something like tracking whether a status was explicitly provided:

var explicitStatus = false
// ...
} else if (type === 'number' && i === 0) {
 status = arg
 explicitStatus = true
}
// ...
if (type === 'object' && arg instanceof Error) {
 err = arg
 if (!explicitStatus) {
 status = err.status || err.statusCode || status
 }
}

This way createError(500, httpError(402)) would correctly return 500, while createError(existingError) without an explicit status would still preserve the error's existing status code — keeping backwards compatibility for that usage.

Would this approach work for a v2 patch, or is this strictly v3 territory?