-
Notifications
You must be signed in to change notification settings - Fork 4
[RUN-3009] Keep track of ReplayScriptAlive and error-out executeCommand calls if its not
#1068
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
Changes from 9 commits
e5b75e7
a192d6b
705e165
1315e01
13cb4ce
f572af8
7fc43d2
2a74949
adfd95e
417048e
e38aab6
97e14e5
7173c87
9fbcd49
b2ca5a8
5996f74
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 | ||||
|---|---|---|---|---|---|---|
|
|
@@ -75,6 +75,7 @@ extern v8::Local<v8::Object> RecordReplayGetBytecode( | |||||
| } // namespace v8 | ||||||
|
|
||||||
| #define CDPERROR_MISSINGCONTEXT 1001 | ||||||
| #define CDPERROR_NOTALIVE 1002 | ||||||
|
|
||||||
| namespace blink { | ||||||
| // using RemoteObjectIdTypeRaw = v8_inspector::String16; | ||||||
|
|
@@ -85,6 +86,7 @@ using RemoteObjectIdTypeRaw = std::u16string; | |||||
| using RemoteObjectIdType = WTF::String; | ||||||
|
|
||||||
| extern "C" void V8RecordReplaySetDefaultContext(v8::Isolate* isolate, v8::Local<v8::Context> cx); | ||||||
| extern "C" int V8RecordReplayGetContextId(v8::Local<v8::Context> cx); | ||||||
| extern "C" void V8RecordReplayFinishRecording(); | ||||||
| extern "C" void V8RecordReplaySetCrashReason(const char* reason); | ||||||
|
|
||||||
|
|
@@ -152,7 +154,7 @@ class InspectorData { | |||||
| LocalFrame* GetLocalFrameRoot() const { return blink::GetLocalFrameRoot(isolate); } | ||||||
| }; | ||||||
|
|
||||||
| static LocalFrame* gLocalRootFrame = nullptr; | ||||||
| static LocalFrame* gRootLocalFrame = nullptr; | ||||||
|
|
||||||
| typedef std::unordered_map<int, InspectorData*> ContextGroupIdInspectorMap; | ||||||
|
|
||||||
|
|
@@ -174,6 +176,7 @@ const { | |||||
| log: log_, | ||||||
| logTrace: logTrace_, | ||||||
| warning: warning_, | ||||||
| fromJsIsReplayScriptAlive: isReplayScriptAlive, | ||||||
| setCDPMessageCallback, | ||||||
| sendCDPMessage, | ||||||
| setCommandCallback, | ||||||
|
|
@@ -198,9 +201,22 @@ const { | |||||
|
|
||||||
| // constants | ||||||
| CDPERROR_MISSINGCONTEXT, | ||||||
| CDPERROR_NOTALIVE, | ||||||
| REPLAY_CDT_PAUSE_OBJECT_GROUP | ||||||
| } = __RECORD_REPLAY_ARGUMENTS__; | ||||||
|
|
||||||
| function log(...args) { | ||||||
| log_(args.join(' ')); | ||||||
| } | ||||||
|
|
||||||
| function logTrace(...args) { | ||||||
| logTrace_(args.join(' ')); | ||||||
| } | ||||||
|
|
||||||
| function warning(...args) { | ||||||
| warning_(args.join(' ')); | ||||||
| } | ||||||
|
|
||||||
| const gSourceMapData = new Map(); | ||||||
|
|
||||||
| try { | ||||||
|
|
@@ -223,18 +239,6 @@ const Array_push = Array.prototype.push; | |||||
| // Some of these are duplicated in gSourceMapScript, so watch out when making | ||||||
| // modifications to update both versions... | ||||||
|
|
||||||
|
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. these comments (and including |
||||||
| function log(...args) { | ||||||
| log_(args.join(' ')); | ||||||
| } | ||||||
|
|
||||||
| function logTrace(...args) { | ||||||
| logTrace_(args.join(' ')); | ||||||
| } | ||||||
|
|
||||||
| function warning(...args) { | ||||||
| warning_(args.join(' ')); | ||||||
| } | ||||||
|
|
||||||
| function assert(v, msg = "") { | ||||||
| if (!v) { | ||||||
| const m = `Assertion failed when handling command (${msg})`; | ||||||
|
|
@@ -325,6 +329,7 @@ class CdpRequest { | |||||
| } | ||||||
|
|
||||||
| const gCdpRequestStack = []; | ||||||
| const gEventListeners = new Map(); | ||||||
|
|
||||||
|
|
||||||
| class CDPMessageError extends Error { | ||||||
|
|
@@ -357,7 +362,7 @@ function sendMessage(method, params) { | |||||
| } | ||||||
| } finally { | ||||||
| const req = gCdpRequestStack.pop(); | ||||||
| assert(req === cdpRequest, "CDP request stack corrupted"); | ||||||
| assert(req === cdpRequest, "[RuntimeError] CDP request stack corrupted"); | ||||||
| } | ||||||
|
|
||||||
| if (cdpRequest.result?.result) { | ||||||
|
|
@@ -369,7 +374,6 @@ function sendMessage(method, params) { | |||||
| return undefined; | ||||||
| } | ||||||
|
|
||||||
| const gEventListeners = new Map(); | ||||||
|
|
||||||
| function addEventListener(method, callback) { | ||||||
| gEventListeners.set(method, callback); | ||||||
|
|
@@ -396,6 +400,7 @@ function messageCallback(message) { | |||||
| is_error: true, | ||||||
| message: e?.message || (e + ''), | ||||||
| stack: e?.stack?.split?.("\n") || e?.stack || [], | ||||||
| code: e?.code, | ||||||
| }); | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -438,7 +443,21 @@ const CommandCallbacks = { | |||||
| "CSS.getAppliedRules": CSS_getAppliedRules | ||||||
| }; | ||||||
|
|
||||||
| function CHECK_ALIVE(message) { | ||||||
| if (!isReplayScriptAlive()) { | ||||||
| const err = new Error(`ReplayScriptContext UNALIVE - ${message}`); | ||||||
| err.code = CDPERROR_NOTALIVE; | ||||||
| throw err; | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| function getAliveLabel() { | ||||||
| return isReplayScriptAlive() ? "" : " [UNALIVE]" | ||||||
| } | ||||||
|
|
||||||
| function executeCommand(method, params) { | ||||||
| CHECK_ALIVE(`executeCommand ${method}`); | ||||||
|
|
||||||
| VerboseCommands && log(`[Command ${method}] Handling command, params=${JSON_stringify(params)}...`); | ||||||
| const result = CommandCallbacks[method](params); | ||||||
| VerboseCommands && log(`[Command ${method}] Handled command, result=${JSON_stringify(result)}`); | ||||||
|
|
@@ -447,21 +466,22 @@ function executeCommand(method, params) { | |||||
|
|
||||||
| function commandCallback(method, params) { | ||||||
| if (!CommandCallbacks[method]) { | ||||||
| log(`[Command ${method}] Missing command callback: ${method}`); | ||||||
| log(`[RuntimeError][Command ${method}] Missing command callback: ${method}`); | ||||||
| return {}; | ||||||
| } | ||||||
|
|
||||||
| try { | ||||||
| return executeCommand(method, params); | ||||||
| } catch (e) { | ||||||
| log(`[RuntimeError][Command ${method}] ${e?.stack || e}`); | ||||||
| log(`[RuntimeError][Command ${method}]${getAliveLabel()} ${e?.stack || e}`); | ||||||
| // Pass the error up to V8; it can (for now) decide how to handle itself, whether | ||||||
| // it should crash or not, etc. Eventually, the caller of the command should make | ||||||
| // that decision. | ||||||
| return { | ||||||
| is_error: true, | ||||||
| message: e?.message || (e + ''), | ||||||
| stack: e?.stack?.split?.("\n") || e?.stack || [], | ||||||
| code: e?.code, | ||||||
| }; | ||||||
| } | ||||||
| } | ||||||
|
|
@@ -655,6 +675,7 @@ function buildRrpObjectResult(cdpReturnValue) { | |||||
| // Sometimes things go wrong. | ||||||
| // E.g. sometimes we get "Cannot find default execution context (-32000) when executing" sendMessage | ||||||
| // from Pause_evaluateIn*. | ||||||
| log(`[RuntimeError] buildRrpObjectResult called without cdpReturnValue ()`); | ||||||
| rrpResult.failed = true; | ||||||
| } | ||||||
| return { result: rrpResult }; | ||||||
|
|
@@ -3202,7 +3223,7 @@ addEventListener("Runtime.executionContextsCleared", () => { | |||||
| sendMessage("Runtime.enable"); | ||||||
|
|
||||||
| } catch (e) { | ||||||
| log(`Error: Initialization exception ${e}`); | ||||||
| warning(`JS_ERROR Initialization: ${e?.stack || e}`); | ||||||
| } | ||||||
|
|
||||||
| })(); | ||||||
|
|
@@ -3891,6 +3912,42 @@ static void LogWarningCallback(const v8::FunctionCallbackInfo<v8::Value>& args) | |||||
| recordreplay::Warning("%s", *text); | ||||||
| } | ||||||
|
|
||||||
| void | ||||||
| RecordReplayRegisterV8Inspector(v8_inspector::V8Inspector* inspector, | ||||||
| v8::Isolate* isolate) { | ||||||
| if (v8::IsMainThread() && IsGReplayScriptEnabled()) { | ||||||
| if (!gV8Inspectors) { | ||||||
| gV8Inspectors = new std::unordered_map<v8::Isolate*,v8_inspector::V8Inspector*>(); | ||||||
| gInspectorData = new std::unordered_map<v8::Isolate*, ContextGroupIdInspectorMap*>(); | ||||||
| } | ||||||
|
|
||||||
| gV8Inspectors->insert(std::make_pair(isolate, inspector)); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| static bool gReplayScriptAlive = false; | ||||||
|
|
||||||
| bool RecordReplayIsReplayScriptAlive() { | ||||||
| return gReplayScriptAlive; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * This is called when gReplayScript's context is about to shut down. | ||||||
| */ | ||||||
| void RecordReplayHandleScriptShutdown(const char* reason, LocalFrame* frame) { | ||||||
| CHECK(v8::IsMainThread()); | ||||||
| if (!gReplayScriptAlive || frame != gRootLocalFrame) { | ||||||
| return; | ||||||
| } | ||||||
| recordreplay::Print("ReplayScriptContext STATUS_CHANGE_UNALIVE - %s", reason); | ||||||
| gReplayScriptAlive = false; | ||||||
| } | ||||||
|
|
||||||
| static void fromJsIsReplayScriptAlive(const v8::FunctionCallbackInfo<v8::Value>& args) { | ||||||
| v8::Isolate* isolate = args.GetIsolate(); | ||||||
| args.GetReturnValue().Set(v8::Number::New(isolate, gReplayScriptAlive)); | ||||||
| } | ||||||
|
|
||||||
| // Function to invoke on CDP responses and events. | ||||||
| static v8::Eternal<v8::Function>* gCDPMessageCallback; | ||||||
|
|
||||||
|
|
@@ -4010,19 +4067,6 @@ v8_inspector::V8InspectorSession* getInspectorSession(v8::Isolate* isolate, int | |||||
| return data->inspectorSession; | ||||||
| } | ||||||
|
|
||||||
| void | ||||||
| RecordReplayRegisterV8Inspector(v8_inspector::V8Inspector* inspector, | ||||||
| v8::Isolate* isolate) { | ||||||
| if (v8::IsMainThread() && IsGReplayScriptEnabled()) { | ||||||
| if (!gV8Inspectors) { | ||||||
| gV8Inspectors = new std::unordered_map<v8::Isolate*,v8_inspector::V8Inspector*>(); | ||||||
| gInspectorData = new std::unordered_map<v8::Isolate*, ContextGroupIdInspectorMap*>(); | ||||||
| } | ||||||
|
|
||||||
| gV8Inspectors->insert(std::make_pair(isolate, inspector)); | ||||||
| } | ||||||
| } | ||||||
|
|
||||||
| static int GetBlinkPersistentId(v8::Local<v8::Object> object) { | ||||||
| v8::Isolate* isolate = v8::Isolate::GetCurrent(); | ||||||
|
|
||||||
|
|
@@ -5501,15 +5545,19 @@ static void InitializeRecordReplayApiObjects(v8::Isolate* isolate, LocalFrame* l | |||||
| v8::Boolean::New(isolate, | ||||||
| TestEnv("RECORD_REPLAY_DISABLE_SOURCEMAP_CACHE"))); | ||||||
|
|
||||||
|
|
||||||
| DefineProperty(isolate, args, "CDPERROR_MISSINGCONTEXT", | ||||||
| v8::Number::New(isolate, (double)CDPERROR_MISSINGCONTEXT)); | ||||||
|
|
||||||
| DefineProperty(isolate, args, "CDPERROR_NOTALIVE", | ||||||
| v8::Number::New(isolate, (double)CDPERROR_NOTALIVE)); | ||||||
|
|
||||||
| SetFunctionProperty(isolate, args, "log", LogCallback); | ||||||
| SetFunctionProperty(isolate, args, "logTrace", LogTraceCallback); | ||||||
| SetFunctionProperty(isolate, args, "warning", LogWarningCallback); | ||||||
|
|
||||||
| // CDP debugger functionality | ||||||
| SetFunctionProperty(isolate, args, "fromJsIsReplayScriptAlive", | ||||||
| fromJsIsReplayScriptAlive); | ||||||
| SetFunctionProperty(isolate, args, "setCDPMessageCallback", | ||||||
| SetCDPMessageCallback); | ||||||
| SetFunctionProperty(isolate, args, "sendCDPMessage", SendCDPMessage); | ||||||
|
|
@@ -5583,13 +5631,8 @@ static void InitializeRecordReplayApiObjects(v8::Isolate* isolate, LocalFrame* l | |||||
| SetFunctionProperty(isolate, args, "checkPersistentId", fromJsCheckPersistentId); | ||||||
| } | ||||||
|
|
||||||
| static void RecordReplaySetDefaultContext(v8::Isolate* isolate, LocalFrame* localFrame, v8::Local<v8::Context> context) { | ||||||
| V8RecordReplaySetDefaultContext(isolate, context); | ||||||
| } | ||||||
|
|
||||||
| void InitializeRecordReplay(v8::Isolate* isolate, LocalFrame* localFrame, v8::Local<v8::Context> context) { | ||||||
| V8RecordReplaySetAPIObjectIdCallback(GetBlinkPersistentId); | ||||||
| RecordReplaySetDefaultContext(isolate, localFrame, context); | ||||||
| gActiveNetworkRequests = | ||||||
| new std::unordered_map<std::string, NetworkRequestStatus>(); | ||||||
| gCurrentNetworkStreamData = new std::vector<uint8_t>(); | ||||||
|
|
@@ -5606,7 +5649,7 @@ static void InitializeReplayScripts(v8::Isolate* isolate, LocalFrame* localFrame | |||||
| // JS stack, we can always use the current root frame's context. | ||||||
| // Note: We are assuming that each tab has its own process, for now. | ||||||
| // (That might not hold true for tabs of the same domain - not sure) | ||||||
| RecordReplaySetDefaultContext(isolate, localFrame, context); | ||||||
| V8RecordReplaySetDefaultContext(isolate, context); | ||||||
|
|
||||||
| // Initialize __RECORD_REPLAY__ things. | ||||||
| InitializeRecordReplayApiObjects(isolate, localFrame); | ||||||
|
|
@@ -5629,6 +5672,7 @@ static void InitializeReplayScripts(v8::Isolate* isolate, LocalFrame* localFrame | |||||
| if (IsGReplayScriptEnabled()) { | ||||||
| recordreplay::AutoMarkReplayCode amrc; | ||||||
| recordreplay::AutoDisallowEvents disallow("InitializeReplayScripts"); | ||||||
|
|
||||||
| // Run `gReplayScript`. | ||||||
| RunScript(isolate, context, gReplayScript, InternalScriptURL); | ||||||
| } | ||||||
|
|
@@ -5644,15 +5688,18 @@ void OnRootFrameInit(v8::Isolate* isolate, LocalFrame* localFrame, v8::Local<v8: | |||||
| localFrame->GetDocument()->Url().GetString().Utf8().c_str() | ||||||
| ); | ||||||
|
|
||||||
| // NOTE: The root `LocalFrame` will actually not change over time. | ||||||
| gLocalRootFrame = localFrame; | ||||||
| // NOTE: The root `LocalFrame` will not necessarily change over time. | ||||||
|
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. I think this isn't a strong enough assertion. how about:
Suggested change
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. Its also less wordy! |
||||||
| gRootLocalFrame = localFrame; | ||||||
|
|
||||||
| // 1. Reset paint surface so that paints to the new root's surface are not ignored. | ||||||
| // See: https://linear.app/replay/issue/RUN-2400 | ||||||
| recordreplay::DoResetPaintSurface(); | ||||||
|
|
||||||
| // 2. Initialize our scripts, command handlers etc. | ||||||
| // 2. Initialize sourcemap worker, command handlers etc. | ||||||
| InitializeReplayScripts(isolate, localFrame, context); | ||||||
|
|
||||||
| gReplayScriptAlive = true; | ||||||
| recordreplay::Print("ReplayScriptContext STATUS_CHANGE_ALIVE"); | ||||||
| } | ||||||
|
|
||||||
| void OnRootFrameInitAfterCheckpoint(v8::Isolate* isolate, LocalFrame* localFrame, v8::Local<v8::Context> context) { | ||||||
|
|
||||||
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.
Note: This did not handle potential cross-domain (and maybe other types of?) navigation where the frame does not get re-used.