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

fetch: fix captureStackTrace #3227

Merged
merged 2 commits into from May 10, 2024
Merged

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented May 8, 2024

Closes #2869

This is an odd bug.

Obviously this is undefined, because nobody will run fetch with the new operator.

So currently if fetch throws, we create a second time the same stacktrace?

Maybe it is wrong and we should set fetchImpl instead of fetch as reference point?

Maybe we can improve the perf, by storing the stackTraceLimit and set it to 0 and after the fetch we reset it. But problem is, that aggregated errors in cause will have no stack trace then?

@Uzlopak
Copy link
Contributor Author

Uzlopak commented May 8, 2024

well, fetch and fetchImpl are not valid values, as it seems. ;)

But I improved the speed of the test, as it was annoying the test it locally and having to wait 10 seconds for each case...

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit 0a64b44 into nodejs:main May 10, 2024
26 checks passed
@Uzlopak Uzlopak deleted the fix-capture-stack-trace branch May 10, 2024 09:57
@github-actions github-actions bot mentioned this pull request May 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

avoid containing undici internals in error stacktrace in web api
2 participants