Preserve stacktrace when wrapping errors

[szmarczak]

Fixes #795

Checklist

• I have read the documentation.
• I have included a pull request description of my changes.
• I have included some tests.
• If it's a new feature, I have included documentation updates.

[szmarczak]

Now:

GotError: foobar
    at get (/home/szm/Desktop/got/source/request-as-event-emitter.ts:250:15)
+   at Object.request (/home/szm/Desktop/got/test/error.ts:206:10)
+   at get (/home/szm/Desktop/got/source/request-as-event-emitter.ts:248:31)
    at Immediate.<anonymous> (/home/szm/Desktop/got/source/request-as-event-emitter.ts:313:4)

Before:

GotError: foobar
    at get (/home/szm/Desktop/got/source/request-as-event-emitter.ts:250:15)
    -- the line above only points to where the error was captured  --
	-- missing what caused the error --
    at Immediate.<anonymous> (/home/szm/Desktop/got/source/request-as-event-emitter.ts:313:4)

[szmarczak]

As you can see, the stacktrace begins with at Immediate.<anonymous>. It's this. But what called it, we don't know. See #447 (comment)

To fix this, we need to either remove setImmediate (no big impact on performance) or capture another stacktrace there (this would have a big impact on performance). Or... leave this as is, because it's default Node behavior.

[sindresorhus]

To fix this, we need to either remove setImmediate (no big impact on performance)

Why do we even have it there? From what I faintly can remember, it had something to do with a race issue. Maybe we could fix it another way and remove the setImmediate?

capture another stacktrace there (this would have a big impact on performance)

I doubt it has big impact on performance relatively to networking.

---

Did you check if the setImmediate problem is the same on Node.js 13 too? Later Node.js versions got much better support for preserving async stack traces.

[szmarczak]

Did you check if the setImmediate problem is the same on Node.js 13 too? Later Node.js versions got much better support for preserving async stack traces.

Yep, it's the same.

I doubt it has big impact on performance relatively to networking.

(() => {
    let timings = [];
    let start;
    for (let i = 0; i < 100000; i++) {
        start = performance.now();
        (new Error()).stack;
        timings.push(performance.now() - start);
    }

	return timings.reduce((a, b) => a + b) / timings.length;
})();

gives me 0.0075 ms / stacktrace. So you're right. I thought it would be time-consuming.

From what I faintly can remember, it had something to do with a race issue. Maybe we could fix it another way and remove the setImmediate?

It was introduced here: #304

We can remove it because the errors are handled in a much different way than at the time.

[szmarczak]

I'll fix this in a few hours, need sleep now

[szmarczak]

We need setImmediate to allow calling promise.json() etc. Promises are executed immediately, so if there were no setImmediate, promise.json() would have no effect on the headers.

[magnusottosson]

@szmarczak Is this supposed to solve the async stack trace problems? On 10.6.0 I get a stack trace that look like this:

HTTPError: Response code 400 (Bad Request) at EventEmitter.<anonymous> ([redacted]/node_modules/got/dist/source/as-promise.js:118:31) at processTicksAndRejections (internal/process/task_queues.js:97:5)

I have no idea where this error occurs :)

My request looks something like this:

const response = await got('someurl', { json: {...}, headers: { 'Cache-Control': 'no-cache, no-store', }, method: 'POST', });

[szmarczak]

@magnusottosson The title of the PR says "when wrapping errors", not "when handling async errors". You're confusing this PR with #1077

[magnusottosson]

Ah sorry. My bad :)