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

Opaque, undebuggable stacktraces #2212

Closed
vedantroy opened this issue Aug 12, 2023 · 8 comments · Fixed by #2215
Closed

Opaque, undebuggable stacktraces #2212

vedantroy opened this issue Aug 12, 2023 · 8 comments · Fixed by #2215
Labels
bug Something isn't working

Comments

@vedantroy
Copy link

Bug Description

When calling await r.json(), if the request text was not valid JSON, undici returns an opaque stacktrace which does not go to the line where the error was happening. This makes debugging near impossible.

Reproducible By

Write a fetch to an endpoint that does not return valid JSON. Then, call fetch("/endpoint").then(r => r.json())

Expected Behavior

An error thrown on the line where await request.json() was called.

Logs & Screenshots

Current behavior:

x-control-1      | SyntaxError: Unexpected token '<', "<!DOCTYPE "... is not valid JSON                                         
x-control-1      |     at JSON.parse (<anonymous>)                                                                              
x-control-1      |     at parseJSONFromBytes (node:internal/deps/undici/undici:6662:19)                                         
x-control-1      |     at successSteps (node:internal/deps/undici/undici:6636:27)                                               
x-control-1      |     at node:internal/deps/undici/undici:1236:60                                                              
x-control-1      |     at node:internal/process/task_queues:140:7                                                               
x-control-1      |     at AsyncResource.runInAsyncScope (node:async_hooks:206:9)                                                
x-control-1      |     at AsyncResource.runMicrotask (node:internal/process/task_queues:137:8)                                  
x-control-1      |     at processTicksAndRejections (node:internal/process/task_queues:95:5)    

Environment

Node 20

@vedantroy vedantroy added the bug Something isn't working label Aug 12, 2023
@vedantroy
Copy link
Author

This also happens when the fetch fails in general.

@ronag
Copy link
Member

ronag commented Aug 13, 2023

@mcollina

@mcollina
Copy link
Member

I don't understand why that line is undebuggable: it seems pretty clear to me.

Is the problem the missing reference to where the call to fetch initiated?

I think this is due to the fact that we are not using the await operator in starting fetch. Wdyt @KhafraDev?

@KhafraDev
Copy link
Member

KhafraDev commented Aug 13, 2023

it's because we use queueMicrotask here

const successSteps = (bytes) => queueMicrotask(() => processBody(bytes))
, probably not much we can do? I also don't really see the value of debugging here because it's not an issue with undici.

@vedantroy
Copy link
Author

Sorry for the lack of clarity here.

The lack of stacktrace is the issue. In a code-base with more than a few fetches, it becomes impossible to tell where the failed fetch occurred since it just gives the error and says nothing about where it originated in the code.

This does not happen with node-fetch, regardless of whether I use await or not.

@mcollina
Copy link
Member

@KhafraDev I think we should be doing something for this, I think it can be very problematic for people.

@KhafraDev
Copy link
Member

do you have any ideas? The queueMicrotask is needed for the spec (as close as a match we can get in node).

@mcollina
Copy link
Member

you'd need to turn

function fullyReadBody (body, processBody, processBodyError) {
into an async function and await readAllBytes. Then inside you'd need to await all those things. If you want to add a microtick, just doing await true would get you there (or await Promise.resolve(), then do await processBody(bytes)

KhafraDev added a commit to KhafraDev/undici that referenced this issue Aug 16, 2023
mcollina pushed a commit that referenced this issue Aug 17, 2023
* better stack trace for body.json

Fixes #2212

* not needed

* nit
crysmags pushed a commit to crysmags/undici that referenced this issue Feb 27, 2024
* better stack trace for body.json

Fixes nodejs#2212

* not needed

* nit
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants