Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

The line where fetch is called doesn't show in the stack trace #1599

Closed
icetbr opened this issue Aug 11, 2022 · 7 comments · Fixed by #1619
Closed

The line where fetch is called doesn't show in the stack trace #1599

icetbr opened this issue Aug 11, 2022 · 7 comments · Fixed by #1619
Labels
enhancement New feature or request fetch good first issue Good for newcomers
Milestone

Comments

@icetbr
Copy link

icetbr commented Aug 11, 2022

Version

v18.7.0

Platform

Linux 98a94d978874 5.4.0-122-generic nodejs/node#138~18.04.1-Ubuntu SMP Fri Jun 24 14:14:03 UTC 2022 x86_64 Linux

Subsystem

fetch

What steps will reproduce the bug?

async function a() {
    await fetch('http://a.com')
}

(async function() {
    await a()
    // → 🎉
}());

How often does it reproduce? Is there a required condition?

Always

What is the expected behavior?

At some point in the stack frame:

 at a (/home/user/projectA/a.js:8:11)

What do you see instead?

TypeError: fetch failed
    at Object.processResponse (node:internal/deps/undici/undici:7175:34)
    at Fetch.fetchFinale (node:internal/deps/undici/undici:7505:21)
    at Fetch.mainFetch (node:internal/deps/undici/undici:7399:21)
    at processTicksAndRejections (node:internal/process/task_queues:96:5) {
  cause: Error: getaddrinfo ENOTFOUND a.com
      at GetAddrInfoReqWrap.onlookup [as oncomplete] (node:dns:82:26) {
    errno: -3008,
    code: 'ENOTFOUND',
    syscall: 'getaddrinfo',
    hostname: 'a.com'
  }
}

Additional information

Maybe undici issue @mcollina ?

@mcollina mcollina transferred this issue from nodejs/node Aug 11, 2022
@mcollina
Copy link
Member

Yes, it's a undici issue, I moved it there.

@mcollina mcollina added enhancement New feature or request fetch labels Aug 11, 2022
@mcollina mcollina added this to the fetch milestone Aug 11, 2022
@mcollina
Copy link
Member

@KhafraDev @ronag have you got some ideas? This would be a true benefit for the ecosystem.

@ronag
Copy link
Member

ronag commented Aug 11, 2022

Doesn't this require some async hook magic?

@mcollina
Copy link
Member

mcollina commented Aug 11, 2022

I don't think it would. This will get us there:

diff --git a/index.js b/index.js
index 8099f5a..0b9a2e5 100644
--- a/index.js
+++ b/index.js
@@ -92,7 +92,12 @@ if (nodeMajor > 16 || (nodeMajor === 16 && nodeMinor >= 8)) {
       fetchImpl = require('./lib/fetch')
     }
     const dispatcher = (arguments[1] && arguments[1].dispatcher) || getGlobalDispatcher()
-    return fetchImpl.apply(dispatcher, arguments)
+    try {
+      return await fetchImpl.apply(dispatcher, arguments)
+    } catch (err) {
+      Error.captureStackTrace(err)
+      throw err
+    }
   }
   module.exports.Headers = require('./lib/fetch/headers').Headers
   module.exports.Response = require('./lib/fetch/response').Response

I'm not sure the exact implications of this.

cc @benjamingr

@mcollina
Copy link
Member

This practically adds 2 microticks.

@ronag ronag added the good first issue Good for newcomers label Aug 22, 2022
@sosoba
Copy link
Contributor

sosoba commented Aug 23, 2022

It is worth paying attention to the second parameter of captureStackTrace. By specifying the scope function (ex. this), you can trim the stack trace from internals.

Error.captureStackTrace(err, this)

@mcollina
Copy link
Member

@sosoba would you like to send a PR to implement this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request fetch good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants