Node's internal errors aren't properly handled by koa's onerror handler in some situations

[julienw]

In some cases, such as running tests in Jest, Node's internal errors such as PREMATURE_CLOSE (from the stream module) aren't properly handled by this code:

koa/lib/context.js

Line 113 in 9ee6584

if (!(err instanceof Error)) err = new Error(util.format('non-error thrown: %j', err));

A bit of debugging showed me that despite that they're Errors, the instanceof Error call returns false. My guess is that they're not using the same Error object (different globals maybe?).

As a result, this is the error I get:

Error: non-error thrown: {}","stack":"Error: non-error thrown: {}
    at Object.onerror (/home/julien/travail/git/profiler-server/node_modules/koa/lib/context.js:115:40)
    at onerror (/home/julien/travail/git/profiler-server/node_modules/koa/lib/application.js:163:32)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)"

This is working properly out of the jest environment though. The same error when running the server directly:

Error [ERR_STREAM_PREMATURE_CLOSE]: Premature close
    at LengthCheckerPassThrough.onclose (internal/streams/end-of-stream.js:61:15)
    at LengthCheckerPassThrough.emit (events.js:215:7)
    at LengthCheckerPassThrough.EventEmitter.emit (domain.js:476:20)
    at emitCloseNT (internal/streams/destroy.js:68:8)
    at processTicksAndRejections (internal/process/task_queues.js:79:21)

But this makes things confusing when finding such an error while working on tests for an app.

Would you be open changing this check using Object.prototype.toString.call(err) !== '[object Error]'? Or maybe use both checks to make sure we catch all cases? Any downsides?

[julienw]

I'm willing to do the patch if this is OK.

[julienw]

There's a problem in jest: jestjs/jest#2549
Other libs fixed it similarly: sindresorhus/execa#423
node itself does it: https://github.com/nodejs/node/blob/81f6dd66fe9e08875e1c96d65b6bce427cfe8b2d/lib/util.js#L99

node proposes util.isError but it's marked as deprecated for a long time (but it's still there!). Newer node versions propose utils.types.isNativeError but this is node >= 7, and I believe koa supports older node for now. That's why I'll just do a patch with Object.prototype.toString.

[anlexN]

@julienw can we use this syntax?
String(err) === 'Error'

[julienw]

I... Don't think this would work ?
What is your problem ?

[anlexN]

i hate old syntax, can we write it in mordern syntax?

[julienw]

Koa supports older node versions so my understanding is that we want to keep using older syntax in this project. Moreover what you suggest doesn't work and I believe is not reliable.

[anlexN]

i just want to know its mordern syntax, can you tell me?