-
Notifications
You must be signed in to change notification settings - Fork 4
[TT-1112] Fix the sourcemap handler script #1229
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
base: master
Are you sure you want to change the base?
Changes from 13 commits
372c8ef
7d09422
caff513
ae03a35
d85d112
9c3ad72
87b23f1
45c09aa
c5ab8c5
2690dda
d5340a8
1077fc2
66dd222
32d7a55
6f5ebaa
95d7b21
86652af
40f37f7
72adac1
82f5fac
862ac54
11be808
3ac5c89
f3dd571
0de85ee
0a5cb9b
a9d1f9a
04001f6
c863175
19e9073
59caef9
887ffe2
3268285
b2c05c3
b9266d1
8a926fe
0ed4689
8f5a0a4
60a1642
6f9da13
189488e
9b89e43
6f09e1c
ae7f0b8
71ed459
c052ec9
2c0df14
561d5ca
4725082
5c0fa0a
122ac49
c9ce2d5
3029c25
faa00ce
a10b104
99cf7f8
d94d8ef
0b42ec6
226d03d
bebbcd6
48aa647
e9a6f0c
19627ca
857cd6a
bb18ed8
52281cb
0c08c9a
6a2d39b
951aede
722f35d
8300b82
cadb278
8b8abec
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -49,7 +49,7 @@ const { | |
| // utils.js | ||
| /////////////////////////////////////////////////////////////////////////////// | ||
|
|
||
| // Some of these are duplicated in gSourceMapScript, so watch out when making | ||
| // Some of these are duplicated in replay_sourcemap_handler, so watch out when making | ||
| // modifications to update both versions... | ||
|
|
||
| function isFunction(val) { | ||
|
|
@@ -422,7 +422,7 @@ function Target_getCurrentMessageContents() { | |
| }; | ||
| } | ||
|
|
||
| addNewScriptHandler((scriptId, sourceURL, relativeSourceMapURL) => { | ||
| addNewScriptHandler("commands", (scriptId, sourceURL, relativeSourceMapURL) => { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| if (!relativeSourceMapURL) | ||
| return; | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,236 @@ | ||
| const { | ||
| log, | ||
| warning, | ||
| getRecordingId, | ||
| sha256DigestHex, | ||
| writeToRecordingDirectory, | ||
| addRecordingEvent, | ||
| addNewScriptHandler, | ||
| getScriptSource, | ||
| recordingDirectoryFileExists, | ||
| readFromRecordingDirectory, | ||
| getRecordingFilePath, | ||
| RECORD_REPLAY_DISABLE_SOURCEMAP_CACHE, | ||
| } = __RECORD_REPLAY_ARGUMENTS__; | ||
|
|
||
| const cache = {}; | ||
|
|
||
| // Provide a cache for urls, salted with the supplied hash. Practically, this | ||
| // means if the script content changes at the url, we will re-download the resource. | ||
| async function getCachedResource(url, hash) { | ||
| const key = `${url}:${hash}`; | ||
| if (cache[key] && !RECORD_REPLAY_DISABLE_SOURCEMAP_CACHE) { | ||
| return cache[key]; | ||
| } | ||
|
|
||
| log(`fetching sourcemap resource ${key}`); | ||
|
|
||
| const res = await fetchText(url); | ||
| cache[key] = res; | ||
| return res; | ||
| } | ||
|
|
||
| addNewScriptHandler("sourcemaps", async (scriptId, sourceURL, relativeSourceMapURL) => { | ||
|
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
|
||
| try { | ||
| if (!relativeSourceMapURL || relativeSourceMapURL.startsWith("data:")) | ||
| return; | ||
|
|
||
| const recordingId = getRecordingId(); | ||
| if (!recordingId) { | ||
| // The recording has been invalidated. | ||
| return; | ||
| } | ||
|
|
||
| const urls = getSourceMapURLs(sourceURL, relativeSourceMapURL); | ||
| if (!urls) | ||
| return; | ||
|
|
||
| const scriptSource = getScriptSource(scriptId); | ||
| const scriptHash = sha256DigestHex(scriptSource); | ||
|
|
||
| const { sourceMapURL, sourceMapBaseURL } = urls; | ||
|
|
||
| let sourceMap; | ||
| try { | ||
| sourceMap = await getCachedResource(sourceMapURL, scriptHash); | ||
| } catch (err) { | ||
| log(`[RuntimeError] Failed to read sourcemap ${sourceMapURL}: ${err.message}`); | ||
| } | ||
| if (!sourceMap) { | ||
| return; | ||
| } | ||
|
|
||
| const id = scriptHash; | ||
| const name = `sourcemap-${id}.map`; | ||
| const lookupName = `sourcemap-${id}.lookup`; | ||
|
|
||
| let sources; | ||
| if (recordingDirectoryFileExists(name) && recordingDirectoryFileExists(lookupName)) { | ||
| try { | ||
| sources = JSON.parse(readFromRecordingDirectory(lookupName)); | ||
| } catch (err) { | ||
| log(`[RuntimeError][sourcemaps] Failed to load sourcemaps from file: ${lookupName} - ${err.message}`); | ||
| } | ||
| } | ||
|
|
||
| if (!sources) { | ||
| writeToRecordingDirectory(name, sourceMap); | ||
|
|
||
| sources = collectUnresolvedSourceMapResources(sourceMap, sourceMapURL, sourceURL); | ||
| writeToRecordingDirectory(lookupName, JSON.stringify(sources)); | ||
| } | ||
|
|
||
| addRecordingEvent(JSON.stringify({ | ||
| kind: "sourcemapAdded", | ||
| path: getRecordingFilePath(name), | ||
| recordingId, | ||
| id, | ||
| url: sourceMapURL, | ||
| baseURL: sourceMapBaseURL, | ||
| targetContentHash: `sha256:${scriptHash}`, | ||
| targetURLHash: sourceURL ? makeAPIHash(sourceURL) : undefined, | ||
| targetMapURLHash: makeAPIHash(sourceMapURL), | ||
| })); | ||
|
|
||
| for (const { offset, url } of sources) { | ||
| let sourceContent; | ||
| try { | ||
| sourceContent = await getCachedResource(url, scriptHash); | ||
| } catch (err) { | ||
| log(`[RuntimeError][sourcemaps] Failed to read original source ${url}: ${err.message}`); | ||
| continue; | ||
| } | ||
| const hash = sha256DigestHex(sourceContent); | ||
| const name = `source-${hash}`; | ||
|
|
||
| if (!recordingDirectoryFileExists(name)) { | ||
| writeToRecordingDirectory(name, sourceContent); | ||
| } | ||
| addRecordingEvent(JSON.stringify({ | ||
| kind: "originalSourceAdded", | ||
| path: getRecordingFilePath(name), | ||
| recordingId, | ||
| parentId: id, | ||
| parentOffset: offset, | ||
| })); | ||
| } | ||
| } catch (err) { | ||
| warning(`[RuntimeError][sourcemaps] Exception - ${err?.stack || err}`); | ||
| } | ||
| }); | ||
|
|
||
| async function fetchText(url) { | ||
| const response = await fetch(url); | ||
| if (!response.ok) { | ||
| throw new Error(`Fetching ${url} failed with status code ${response.status} (${response.statusText})`); | ||
| } | ||
| return await response.text(); | ||
| } | ||
|
|
||
| function makeAPIHash(content) { | ||
| assert(typeof content === "string"); | ||
| const digestHex = sha256DigestHex(content); | ||
| return "sha256:" + digestHex; | ||
| } | ||
|
|
||
| function collectUnresolvedSourceMapResources(mapText, mapURL) { | ||
| let obj; | ||
| let sourceOffset = 0; | ||
|
|
||
| function logError(msg) { | ||
| log(`[RuntimeError][sourcemaps] ${msg} (${mapURL}:${sourceOffset})`); | ||
| } | ||
|
|
||
| try { | ||
| obj = JSON.parse(mapText); | ||
| if (typeof obj !== "object" || !obj) { | ||
| return []; | ||
| } | ||
| } catch (err) { | ||
| logError(`Exception parsing sourcemap JSON (${mapURL}): ${err?.message || err}`); | ||
| return []; | ||
| } | ||
|
|
||
| const unresolvedSources = []; | ||
| if (obj.version !== 3) { | ||
| logError("Invalid sourcemap version: " + obj.version); | ||
| return []; | ||
| } | ||
|
|
||
| if (obj.sources != null) { | ||
| const { sourceRoot, sources, sourcesContent } = obj; | ||
|
|
||
| if (Array.isArray(sources)) { | ||
| for (let i = 0; i < sources.length; i++) { | ||
| const offset = sourceOffset++; | ||
|
|
||
| if ( | ||
| !Array.isArray(sourcesContent) || | ||
| typeof sourcesContent[i] !== "string" | ||
| ) { | ||
| let url = sources[i]; | ||
| if (typeof sourceRoot === "string" && sourceRoot) { | ||
| url = sourceRoot.replace(/\/?/, "/") + url; | ||
| } | ||
| let sourceURL; | ||
| try { | ||
| sourceURL = new URL(url, mapURL).toString(); | ||
| } catch { | ||
| logError("Unable to compute original source URL: " + url); | ||
| continue; | ||
| } | ||
|
|
||
| unresolvedSources.push({ | ||
| offset, | ||
| url: sourceURL, | ||
| }); | ||
| } | ||
| } | ||
| } else { | ||
| logError("Invalid sourcemap sources list"); | ||
| } | ||
| } | ||
|
|
||
| return unresolvedSources; | ||
| } | ||
|
|
||
| function assert(v, msg = "") { | ||
| if (!v) { | ||
| const m = `Assertion failed when handling command (${msg})`; | ||
| log(`[RuntimeError] ${m} - ${Error().stack}`); | ||
| throw new Error(m); | ||
| } | ||
| } | ||
|
|
||
| function getSourceMapURLs(sourceURL, relativeSourceMapURL) { | ||
| let sourceBaseURL; | ||
| if (typeof sourceURL === "string" && isValidBaseURL(sourceURL)) { | ||
| sourceBaseURL = sourceURL; | ||
| } else if (window?.location?.href && isValidBaseURL(window?.location?.href)) { | ||
| sourceBaseURL = window.location.href; | ||
| } | ||
|
|
||
| let sourceMapURL; | ||
| try { | ||
| sourceMapURL = new URL(relativeSourceMapURL, sourceBaseURL).toString(); | ||
| } catch (err) { | ||
| log("Failed to process sourcemap url: " + err.message); | ||
| return null; | ||
| } | ||
|
|
||
| // If the map was a data: URL or something along those lines, we want | ||
| // to resolve paths in the map relative to the overall base. | ||
| const sourceMapBaseURL = | ||
| isValidBaseURL(sourceMapURL) ? sourceMapURL : sourceBaseURL; | ||
|
|
||
| return { sourceMapURL, sourceMapBaseURL }; | ||
| } | ||
|
|
||
| function isValidBaseURL(url) { | ||
| try { | ||
| new URL("", url); | ||
| return true; | ||
| } catch { | ||
| return false; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -24,7 +24,7 @@ function findMatches(text /*: string*/, regex /*: RegExp*/) { | |
| function extractNamedScriptBlock( | ||
| text /*: string*/, | ||
| start /*: number*/, | ||
| end /*: number*/, | ||
| end /*: number*/ | ||
|
Domiii marked this conversation as resolved.
|
||
| ) { | ||
| const lines = text.split("\n"); | ||
| const name = lines[start].split(" ")[2]; | ||
|
|
@@ -66,7 +66,10 @@ function calculateStatsPerFile(messages) { | |
| return stat; | ||
| } | ||
|
|
||
| async function lintScript(fpath, { name, text } /*: { name: string, text: string }*/) { | ||
| async function lintScript( | ||
| fpath, | ||
| { name, text } /*: { name: string, text: string }*/ | ||
| ) { | ||
| const messages = linter.verify(text, { | ||
| parserOptions: { | ||
| ecmaVersion: 2023, | ||
|
|
@@ -135,22 +138,24 @@ async function lintScript(fpath, { name, text } /*: { name: string, text: string | |
| return { errorCount, fatalErrorCount, warningCount }; | ||
| } | ||
|
|
||
| const AssetFiles = [ | ||
| "replay_command_handlers.js", | ||
| "replay_sourcemap_handler.js", | ||
| ]; | ||
|
|
||
|
Comment on lines
+141
to
+146
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. is this (coupled with the block 156-158 below) the only change to this file?
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Should be! |
||
| async function main() { | ||
| await lintFile( | ||
| path.join( | ||
| ROOT_DIR, | ||
| "third_party/blink/renderer/bindings/core/v8/record_replay_interface.cc", | ||
| "third_party/blink/renderer/bindings/core/v8/record_replay_interface.cc" | ||
| ), | ||
| /R""""\(/g, | ||
| /\)""""/g | ||
| ); | ||
|
|
||
| await lintFile( | ||
| path.join( | ||
| ROOT_DIR, | ||
| "replay-assets/replay_command_handlers.js", | ||
| ) | ||
| ); | ||
| for (const jsFile of AssetFiles) { | ||
| await lintFile(path.join(ROOT_DIR, "replay-assets/" + jsFile)); | ||
| } | ||
| } | ||
|
|
||
| async function lintFile(fpath, startRegex, endRegex) { | ||
|
|
@@ -161,15 +166,19 @@ async function lintFile(fpath, startRegex, endRegex) { | |
| const lineNumbers = findMatches(replayText, startRegex); | ||
| const endLineNumbers = findMatches(replayText, endRegex); | ||
| if (lineNumbers?.length != endLineNumbers?.length) { | ||
| throw new Error(`Lint failed in ${fpath} - start and end line numbers don't match: ${lineNumbers?.length} != ${endLineNumbers?.length}`); | ||
| throw new Error( | ||
| `Lint failed in ${fpath} - start and end line numbers don't match: ${lineNumbers?.length} != ${endLineNumbers?.length}` | ||
| ); | ||
| } | ||
| // console.log("lintFile", lineNumbers?.length, endLineNumbers?.length); | ||
| jsTextBlocks = lineNumbers.map((lineNumber, index) => | ||
| extractNamedScriptBlock(replayText, lineNumber, endLineNumbers[index]), | ||
| extractNamedScriptBlock(replayText, lineNumber, endLineNumbers[index]) | ||
| ); | ||
|
|
||
| if (!jsTextBlocks.length) { | ||
| throw new Error(`Invalid regexes or file path. Could not find js text block in ${fpath}.`); | ||
| throw new Error( | ||
| `Invalid regexes or file path. Could not find js text block in ${fpath}.` | ||
| ); | ||
| } | ||
| } else { | ||
| jsTextBlocks = [{ text: replayText }]; | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
is this related to this change or TT-1150?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The latter. But also: Its just a bug in this function 🤷♀️