Skip to content

Commit 44f20fc

Browse files
author
bcoe
committed
errors: do not call resolve on URLs with schemes
Based on advice from webpack project, this PR stops trying to resolve sources with a webpack:// scheme (or other schemes, as they represent absolute URLs). Source content is now loaded from the sourcesContent lookup, if it is populated (allowing for non file:// schemes). Refs: #35325
1 parent b92d2e4 commit 44f20fc

File tree

5 files changed

+52
-15
lines changed

5 files changed

+52
-15
lines changed

lib/internal/source_map/prepare_stack_trace.js

Lines changed: 28 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -40,14 +40,12 @@ const prepareStackTrace = (globalThis, error, trace) => {
4040
}
4141

4242
let errorSource = '';
43-
let firstSource;
4443
let firstLine;
4544
let firstColumn;
4645
const preparedTrace = trace.map((t, i) => {
4746
if (i === 0) {
4847
firstLine = t.getLineNumber();
4948
firstColumn = t.getColumnNumber();
50-
firstSource = t.getFileName();
5149
}
5250
let str = i !== 0 ? '\n at ' : '';
5351
str = `${str}${t}`;
@@ -63,16 +61,21 @@ const prepareStackTrace = (globalThis, error, trace) => {
6361
} = sm.findEntry(t.getLineNumber() - 1, t.getColumnNumber() - 1);
6462
if (originalSource && originalLine !== undefined &&
6563
originalColumn !== undefined) {
66-
const originalSourceNoScheme = originalSource
67-
.replace(/^file:\/\//, '');
6864
if (i === 0) {
6965
firstLine = originalLine + 1;
7066
firstColumn = originalColumn + 1;
71-
firstSource = originalSourceNoScheme;
67+
7268
// Show error in original source context to help user pinpoint it:
73-
errorSource = getErrorSource(firstSource, firstLine, firstColumn);
69+
errorSource = getErrorSource(
70+
sm.payload,
71+
originalSource,
72+
firstLine,
73+
firstColumn
74+
);
7475
}
7576
// Show both original and transpiled stack trace information:
77+
const originalSourceNoScheme = originalSource
78+
.replace(/^file:\/\//, '');
7679
str += `\n -> ${originalSourceNoScheme}:${originalLine + 1}:` +
7780
`${originalColumn + 1}`;
7881
}
@@ -88,15 +91,26 @@ const prepareStackTrace = (globalThis, error, trace) => {
8891
// Places a snippet of code from where the exception was originally thrown
8992
// above the stack trace. This logic is modeled after GetErrorSource in
9093
// node_errors.cc.
91-
function getErrorSource(firstSource, firstLine, firstColumn) {
94+
function getErrorSource(payload, originalSource, firstLine, firstColumn) {
9295
let exceptionLine = '';
96+
const originalSourceNoScheme = originalSource.replace(/^file:\/\//, '');
97+
9398
let source;
94-
try {
95-
source = readFileSync(firstSource, 'utf8');
96-
} catch (err) {
97-
debug(err);
98-
return exceptionLine;
99+
const sourceContentIndex = payload.sources.indexOf(originalSource);
100+
if (payload.sourcesContent?.[sourceContentIndex]) {
101+
// First we check if the original source content was provided in the
102+
// source map itself:
103+
source = payload.sourcesContent[sourceContentIndex];
104+
} else {
105+
// If no sourcesContent was found, attempt to load the original source
106+
// from disk:
107+
try {
108+
source = readFileSync(originalSourceNoScheme, 'utf8');
109+
} catch (err) {
110+
debug(err);
111+
}
99112
}
113+
100114
const lines = source.split(/\r?\n/, firstLine);
101115
const line = lines[firstLine - 1];
102116
if (!line) return exceptionLine;
@@ -110,7 +124,8 @@ function getErrorSource(firstSource, firstLine, firstColumn) {
110124
}
111125
prefix = prefix.slice(0, -1); // The last character is the '^'.
112126

113-
exceptionLine = `${firstSource}:${firstLine}\n${line}\n${prefix}^\n\n`;
127+
exceptionLine =
128+
`${originalSourceNoScheme}:${firstLine}\n${line}\n${prefix}^\n\n`;
114129
return exceptionLine;
115130
}
116131

lib/internal/source_map/source_map_cache.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -165,14 +165,16 @@ function sourceMapFromDataUrl(basePath, url) {
165165
// If the sources are not absolute URLs after prepending of the "sourceRoot",
166166
// the sources are resolved relative to the SourceMap (like resolving script
167167
// src in a html document).
168+
// TODO(bcoe): refactor to use new URL(url, base), rather than path.resolve.
168169
function sourcesToAbsolute(base, data) {
169170
data.sources = data.sources.map((source) => {
170171
source = (data.sourceRoot || '') + source;
172+
// An absolute URI does not need to be resolved, return it:
173+
if (source.match(/(?<scheme>^\w+):\/\//)) return source;
171174
if (!/^[\\/]/.test(source[0])) {
172175
source = resolve(base, source);
173176
}
174-
if (!source.startsWith('file://')) source = `file://${source}`;
175-
return source;
177+
return `file://${source}`;
176178
});
177179
// The sources array is now resolved to absolute URLs, sourceRoot should
178180
// be updated to noop.

test/fixtures/source-map/webpack.js

Lines changed: 2 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/fixtures/source-map/webpack.js.map

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

test/parallel/test-source-map-enable.js

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,23 @@ function nextdir() {
265265
);
266266
}
267267

268+
// Does not attempt to apply path resolution logic to absolute URLs
269+
// with schemes.
270+
// Refs: https://github.com/webpack/webpack/issues/9601
271+
// Refs: https://sourcemaps.info/spec.html#h.75yo6yoyk7x5
272+
{
273+
const output = spawnSync(process.execPath, [
274+
'--enable-source-maps',
275+
require.resolve('../fixtures/source-map/webpack.js')
276+
]);
277+
// Error in original context of source content:
278+
assert.ok(
279+
output.stderr.toString().match(/throw new Error\('oh no!'\)\n.*\^/)
280+
);
281+
// Rewritten stack trace:
282+
assert.ok(output.stderr.toString().includes('webpack:///./webpack.js:14:9'));
283+
}
284+
268285
function getSourceMapFromCache(fixtureFile, coverageDirectory) {
269286
const jsonFiles = fs.readdirSync(coverageDirectory);
270287
for (const jsonFile of jsonFiles) {

0 commit comments

Comments
 (0)