-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
[v14.x backport] errors: do not call resolve on URLs with schemes #37717
Conversation
We were incorrectly trying to run path.resolve on absolute sources URLs. This was breaking webpack:// URLs in stack trace output. Refs: nodejs#35325 Backport-PR-URL: nodejs#37717 PR-URL: nodejs#35903 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
8cae83d
to
1cde0f9
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
35bd63e
to
1cde0f9
Compare
1cde0f9
to
2197404
Compare
We were incorrectly trying to run path.resolve on absolute sources URLs. This was breaking webpack:// URLs in stack trace output. Refs: nodejs#35325 Backport-PR-URL: nodejs#37717 PR-URL: nodejs#35903 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
2197404
to
f91f140
Compare
@aduh95 any idea where to go next with the |
Sorry I can't explain the failure either, it's really weird. |
@bcoe is this same target failing the same way if you try running them in |
@ruyadorno v14.x-staging seems fine, so something about this PR is breaking the test. The weird thing is the test isn't failing on test-asan on the main branch either (which includes this change). We might need to try to reproduce in a container locally. |
5c95073
to
0e1a9b5
Compare
We were incorrectly trying to run path.resolve on absolute sources URLs. This was breaking webpack:// URLs in stack trace output. Refs: nodejs#35325 Backport-PR-URL: nodejs#37717 PR-URL: nodejs#35903 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
e59434d
to
e4ad78b
Compare
@targos @richardlau I've been unable to reproduce the I noticed in #38001 we also landed a PR to 14.x-staging with test-asan failures, is it alright if I do likewise with this PR? (perhaps with a tracking issue to dig into |
11505ad
to
6b115d7
Compare
We were incorrectly trying to run path.resolve on absolute sources URLs. This was breaking webpack:// URLs in stack trace output. Refs: nodejs#35325 PR-URL: nodejs#35903 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
e4ad78b
to
753d783
Compare
We were incorrectly trying to run path.resolve on absolute sources URLs. This was breaking webpack:// URLs in stack trace output. Refs: #35325 PR-URL: #35903 Backport-PR-URL: #37717 Reviewed-By: Rich Trott <[email protected]> Reviewed-By: Antoine du Hamel <[email protected]> Reviewed-By: Matteo Collina <[email protected]>
Landed in b01c496 |
We were incorrectly trying to run path.resolve on absolute
sources URLs. This was breaking webpack:// URLs in stack
trace output.
Refs: #35325
PR-URL: #35903
Reviewed-By: Rich Trott [email protected]
Reviewed-By: Antoine du Hamel [email protected]
Reviewed-By: Matteo Collina [email protected]