Skip to content

Commit 72ad567

Browse files
Merge pull request #1192 from gemini-testing/INFRADUTY-29746.migrate_sourcemap_lib
fix: incorrect err snippet position in esm files
2 parents 75ee712 + dba949a commit 72ad567

File tree

8 files changed

+98
-77
lines changed

8 files changed

+98
-77
lines changed

package-lock.json

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

package.json

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -104,7 +104,7 @@
104104
"sizzle": "2.3.6",
105105
"socket.io": "4.7.5",
106106
"socket.io-client": "4.7.5",
107-
"source-map": "0.7.6",
107+
"source-map-js": "1.2.1",
108108
"strftime": "0.10.2",
109109
"strip-ansi": "6.0.1",
110110
"temp": "0.8.3",

src/browser/cdp/selectivity/json-utils.ts

Lines changed: 5 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
1-
import fs from "node:fs";
1+
import path from "node:path";
22
import zlib from "node:zlib";
3+
import fs from "fs-extra";
34
import { Compression, type SelectivityCompressionType } from "./types";
45
import type { Readable, Transform, Writable } from "node:stream";
56

@@ -154,13 +155,15 @@ export const readJsonWithCompression = async <T>(
154155
return JSON.parse(fileData);
155156
};
156157

157-
export const writeJsonWithCompression = (
158+
export const writeJsonWithCompression = async (
158159
jsonBasePath: string,
159160
data: unknown,
160161
preferredCompressionType: SelectivityCompressionType,
161162
): Promise<void> => {
162163
const filePath = jsonBasePath + getCompressionPrefix(preferredCompressionType);
163164
const fileData = JSON.stringify(data, null, preferredCompressionType === "none" ? 2 : 0);
164165

166+
await fs.ensureDir(path.dirname(filePath));
167+
165168
return writeCompressedTextFile(filePath, fileData, preferredCompressionType);
166169
};

src/browser/cdp/selectivity/utils.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import { sortedIndex, memoize } from "lodash";
2-
import { SourceMapConsumer, type BasicSourceMapConsumer, type RawSourceMap } from "source-map";
2+
import { SourceMapConsumer, type RawSourceMap } from "source-map-js";
33
import fs from "fs";
44
import path from "path";
55
import { URL } from "url";
@@ -89,7 +89,7 @@ export const extractSourceFilesDeps = async (
8989
const dependantSourceFiles = new Set<string>();
9090
const sourceMapsParsed = patchSourceMapSources(JSON.parse(sourceMaps), sourceRoot);
9191

92-
const consumer = (await new SourceMapConsumer(sourceMapsParsed)) as BasicSourceMapConsumer;
92+
const consumer = new SourceMapConsumer(sourceMapsParsed);
9393

9494
let sourceOffset = source.indexOf("\n");
9595
const offsetToLine = [0];

src/error-snippets/source-maps.ts

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,16 +1,16 @@
1-
import { SourceMapConsumer, type BasicSourceMapConsumer } from "source-map";
1+
import { RawSourceMap, SourceMapConsumer } from "source-map-js";
22
import url from "url";
33
import { JS_SOURCE_MAP_URL_COMMENT } from "./constants";
44
import { getSourceCodeFile } from "./utils";
55
import { softFileURLToPath } from "../utils/fs";
66
import { transformCode } from "../utils/typescript";
77
import type { SufficientStackFrame, ResolvedFrame } from "./types";
88

9-
export const extractSourceMaps = async (
10-
fileContents: string,
11-
fileName: string,
12-
): Promise<BasicSourceMapConsumer | null> => {
13-
if (fileContents.indexOf(JS_SOURCE_MAP_URL_COMMENT) === -1) {
9+
export const extractSourceMaps = async (fileContents: string, fileName: string): Promise<SourceMapConsumer | null> => {
10+
const hasNoSourceMaps = fileContents.indexOf(JS_SOURCE_MAP_URL_COMMENT) === -1;
11+
const isEsmFile = fileName.startsWith("file://");
12+
13+
if (hasNoSourceMaps && !isEsmFile) {
1414
fileContents = transformCode(fileContents, { sourceFile: fileName, sourceMaps: true });
1515
}
1616

@@ -27,16 +27,16 @@ export const extractSourceMaps = async (
2727
: fileContents.slice(sourceMapsStartIndex + JS_SOURCE_MAP_URL_COMMENT.length, sourceMapsEndIndex);
2828

2929
const sourceMaps = await getSourceCodeFile(url.resolve(fileName, sourceMapUrl));
30-
const consumer = (await new SourceMapConsumer(sourceMaps)) as BasicSourceMapConsumer;
30+
const rawSourceMaps = JSON.parse(sourceMaps) as RawSourceMap;
3131

32-
consumer.file = consumer.file || fileName;
32+
rawSourceMaps.file = rawSourceMaps.file || fileName;
3333

34-
return consumer;
34+
return new SourceMapConsumer(rawSourceMaps);
3535
};
3636

3737
export const resolveLocationWithSourceMap = (
3838
stackFrame: SufficientStackFrame,
39-
sourceMaps: BasicSourceMapConsumer,
39+
sourceMaps: SourceMapConsumer,
4040
): ResolvedFrame => {
4141
const positions = sourceMaps.originalPositionFor({ line: stackFrame.lineNumber, column: stackFrame.columnNumber });
4242
const source = positions.source ? sourceMaps.sourceContentFor(positions.source) : null;
@@ -50,5 +50,5 @@ export const resolveLocationWithSourceMap = (
5050
throw new Error("Line and column could not be evaluated from the source map");
5151
}
5252

53-
return { file: softFileURLToPath(sourceMaps.file), source, location };
53+
return { file: softFileURLToPath(sourceMaps.file as string), source, location };
5454
};

test/src/browser/cdp/selectivity/utils.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ describe("CDP/Selectivity/Utils", () => {
4444
utils = proxyquire("src/browser/cdp/selectivity/utils", {
4545
fs: fsStub,
4646
path: pathStub,
47-
"source-map": {
47+
"source-map-js": {
4848
SourceMapConsumer: SourceMapConsumerStub,
4949
},
5050
"../../../utils/fs": {
@@ -130,7 +130,7 @@ describe("CDP/Selectivity/Utils", () => {
130130
describe("patchSourceMapSources", () => {
131131
it("should patch webpack protocol sources", () => {
132132
const sourceMap = {
133-
version: 3,
133+
version: 3 as unknown as string,
134134
sources: ["webpack://src/app.js", "webpack://src/utils.js", "regular/file.js"],
135135
sourceRoot: "",
136136
names: [],
@@ -146,7 +146,7 @@ describe("CDP/Selectivity/Utils", () => {
146146

147147
it("should use existing sourceRoot if no custom sourceRoot provided", () => {
148148
const sourceMap = {
149-
version: 3,
149+
version: 3 as unknown as string,
150150
sources: ["webpack:///src/app.js"],
151151
sourceRoot: "/existing/root",
152152
names: [],
@@ -161,7 +161,7 @@ describe("CDP/Selectivity/Utils", () => {
161161

162162
it("should handle sources without webpack protocol", () => {
163163
const sourceMap = {
164-
version: 3,
164+
version: 3 as unknown as string,
165165
sources: ["src/app.js", "lib/utils.js"],
166166
sourceRoot: "",
167167
names: [],
@@ -528,7 +528,7 @@ describe("CDP/Selectivity/Utils", () => {
528528
utils = proxyquire("src/browser/cdp/selectivity/utils", {
529529
fs: fsStub,
530530
path: { ...pathStub, join: pathJoinStub },
531-
"source-map": {
531+
"source-map-js": {
532532
SourceMapConsumer: SourceMapConsumerStub,
533533
},
534534
"../../../utils/fs": {
@@ -609,7 +609,7 @@ describe("CDP/Selectivity/Utils", () => {
609609
utils = proxyquire("src/browser/cdp/selectivity/utils", {
610610
fs: fsStub,
611611
path: { ...pathStub, join: pathJoinStub },
612-
"source-map": {
612+
"source-map-js": {
613613
SourceMapConsumer: SourceMapConsumerStub,
614614
},
615615
"../../../utils/fs": {

test/src/error-snippets/index.ts

Lines changed: 25 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -25,17 +25,27 @@ describe("error-snippets", () => {
2525
let fetchStub: SinonStub;
2626
let formatErrorSnippetStub: SinonStub;
2727
let findRelevantStackFrameStub: SinonStub;
28+
let extractSourceMapsStub: SinonStub;
29+
let resolveLocationWithSourceMap: SinonStub;
2830

2931
beforeEach(() => {
3032
fsReadFileStub = sandbox.stub(fs, "readFile");
31-
fetchStub = sandbox.stub(globalThis, "fetch");
32-
33+
fetchStub = sandbox.stub(globalThis, "fetch").resolves({
34+
text: () => Promise.resolve("source code"),
35+
headers: new Map() as unknown as Headers,
36+
} as Response);
3337
formatErrorSnippetStub = sandbox.stub();
3438
findRelevantStackFrameStub = sandbox.stub();
39+
extractSourceMapsStub = sandbox.stub().resolves(null);
40+
resolveLocationWithSourceMap = sandbox.stub();
3541

3642
extendWithCodeSnippet = proxyquire("../../../src/error-snippets", {
3743
"./utils": { formatErrorSnippet: formatErrorSnippetStub },
3844
"./frames": { findRelevantStackFrame: findRelevantStackFrameStub },
45+
"./source-maps": {
46+
extractSourceMaps: extractSourceMapsStub,
47+
resolveLocationWithSourceMap: resolveLocationWithSourceMap,
48+
},
3949
}).extendWithCodeSnippet;
4050
});
4151

@@ -97,27 +107,35 @@ describe("error-snippets", () => {
97107
assert.equal(error, result);
98108
});
99109

100-
it("should return error with snippet for file path", async () => {
110+
it("should return error with snippet for file path for esm file from stacktrace", async () => {
101111
const error = new Error("test error");
102-
const stackFrame = { fileName: "/file/file1", lineNumber: 100, columnNumber: 500 };
112+
const stackFrame = { fileName: "file:///file/file1", lineNumber: 100, columnNumber: 500 };
103113
findRelevantStackFrameStub.returns(stackFrame);
114+
extractSourceMapsStub.resolves(null);
104115
formatErrorSnippetStub.returns("code snippet");
105-
fsReadFileStub.resolves("source code");
116+
fsReadFileStub.resolves("async function main() {}");
106117

107118
const result = await extendWithCodeSnippet(error);
108119

109120
assert.equal(result.snippet, "code snippet");
110121
});
111122

112-
it("should return error with snippet for network url", async () => {
123+
it("should return error with snippet for network url from source maps", async () => {
113124
const error = new Error("test error");
114125
const stackFrame = { fileName: "http://localhost:3000/file/file1", lineNumber: 100, columnNumber: 500 };
126+
const resolvedLocation = {
127+
file: "file/file1",
128+
source: "source code",
129+
location: {},
130+
};
115131
findRelevantStackFrameStub.returns(stackFrame);
116-
formatErrorSnippetStub.returns("code snippet");
117132
fetchStub.withArgs("http://localhost:3000/file/file1").resolves({
118133
text: () => Promise.resolve("source code"),
119134
headers: new Map(),
120135
});
136+
extractSourceMapsStub.resolves("source-maps");
137+
resolveLocationWithSourceMap.withArgs(stackFrame, "source-maps").resolves(resolvedLocation);
138+
formatErrorSnippetStub.withArgs(error, resolvedLocation).returns("code snippet");
121139

122140
const result = await extendWithCodeSnippet(error);
123141

test/src/error-snippets/source-maps.ts

Lines changed: 41 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import sinon, { type SinonStub } from "sinon";
2-
import { SourceMapConsumer, type BasicSourceMapConsumer, type NullableMappedPosition } from "source-map";
2+
import { MappedPosition, SourceMapConsumer } from "source-map-js";
33
import { extractSourceMaps, resolveLocationWithSourceMap } from "./../../../src/error-snippets/source-maps";
44
import type { SufficientStackFrame, ResolvedFrame } from "../../../src/error-snippets/types";
55

@@ -17,13 +17,28 @@ describe("error-snippets/source-maps", () => {
1717
describe("extractSourceMaps", () => {
1818
it("should return null if source maps comment is not present in file content", async () => {
1919
const fileContents = 'console.log("Hello, World!");';
20-
const fileName = "test.js";
20+
const fileName = "file://test.js";
2121

2222
const result = await extractSourceMaps(fileContents, fileName);
2323

2424
assert.isNull(result);
2525
});
2626

27+
it("should return null if file has esm path", async () => {
28+
const inlineSourceMap =
29+
"data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IiJ9";
30+
const fileContents = `console.log("Hello, World!");\n//# sourceMappingURL=${inlineSourceMap}`;
31+
const fileName = "file:///some/path/test.js";
32+
fetchStub.withArgs(inlineSourceMap).resolves({
33+
text: () => Promise.resolve('{"version":3,"sources":[],"names":[],"mappings":""}'),
34+
headers: new Map(),
35+
});
36+
37+
const result = await extractSourceMaps(fileContents, fileName);
38+
39+
assert.instanceOf(result, SourceMapConsumer);
40+
});
41+
2742
it("should return a SourceMapConsumer instance if source maps comment is present in file content", async () => {
2843
const inlineSourceMap =
2944
"data:application/json;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbXSwibmFtZXMiOltdLCJtYXBwaW5ncyI6IiJ9";
@@ -42,14 +57,12 @@ describe("error-snippets/source-maps", () => {
4257

4358
describe("resolveLocationWithSourceMap", () => {
4459
it("should throw an error when source is null", async () => {
45-
const sourceMaps = (await new SourceMapConsumer(
46-
JSON.stringify({
47-
version: 3,
48-
sources: [],
49-
names: [],
50-
mappings: "",
51-
}),
52-
)) as BasicSourceMapConsumer;
60+
const sourceMaps = new SourceMapConsumer({
61+
version: 3 as unknown as string,
62+
sources: [],
63+
names: [],
64+
mappings: "",
65+
});
5366
const stackFrame = { lineNumber: 5, columnNumber: 10 } as SufficientStackFrame;
5467

5568
const fn = (): ResolvedFrame => resolveLocationWithSourceMap(stackFrame, sourceMaps);
@@ -58,16 +71,15 @@ describe("error-snippets/source-maps", () => {
5871
});
5972

6073
it("should throw an error when line or column is null", async () => {
61-
const sourceMaps = (await new SourceMapConsumer(
62-
JSON.stringify({
63-
version: 3,
64-
sources: ["file1"],
65-
names: [],
66-
mappings: "",
67-
sourcesContent: ["content"],
68-
}),
69-
)) as BasicSourceMapConsumer;
70-
sandbox.stub(sourceMaps, "originalPositionFor").returns({ source: "file1" } as NullableMappedPosition);
74+
const sourceMaps = new SourceMapConsumer({
75+
// Specification says it should be number
76+
version: 3 as unknown as string,
77+
sources: ["file1"],
78+
names: [],
79+
mappings: "",
80+
sourcesContent: ["content"],
81+
});
82+
sandbox.stub(sourceMaps, "originalPositionFor").returns({ source: "file1" } as MappedPosition);
7183
const stackFrame = { lineNumber: 5, columnNumber: 10 } as SufficientStackFrame;
7284

7385
const fn = (): ResolvedFrame => resolveLocationWithSourceMap(stackFrame, sourceMaps);
@@ -76,19 +88,17 @@ describe("error-snippets/source-maps", () => {
7688
});
7789

7890
it("should return ResolvedFrame", async () => {
79-
const sourceMaps = (await new SourceMapConsumer(
80-
JSON.stringify({
81-
version: 3,
82-
sources: ["file1"],
83-
names: [],
84-
mappings: "AAAA;AACA",
85-
sourcesContent: ["content"],
86-
}),
87-
)) as BasicSourceMapConsumer;
88-
sourceMaps.file = "file:///file1";
91+
const sourceMaps = new SourceMapConsumer({
92+
version: 3 as unknown as string,
93+
sources: ["file1"],
94+
names: [],
95+
mappings: "AAAA;AACA",
96+
sourcesContent: ["content"],
97+
file: "file:///file1",
98+
});
8999
sandbox
90100
.stub(sourceMaps, "originalPositionFor")
91-
.returns({ source: "file1", line: 100, column: 500 } as NullableMappedPosition);
101+
.returns({ source: "file1", line: 100, column: 500 } as MappedPosition);
92102
const stackFrame = { lineNumber: 1, columnNumber: 1 } as SufficientStackFrame;
93103

94104
const result = resolveLocationWithSourceMap(stackFrame, sourceMaps);

0 commit comments

Comments
 (0)