diff --git a/core/gather/driver/execution-context.js b/core/gather/driver/execution-context.js index ba8b768a449b..1045f7449649 100644 --- a/core/gather/driver/execution-context.js +++ b/core/gather/driver/execution-context.js @@ -80,15 +80,10 @@ class ExecutionContext { * page without isolation. * @param {string} expression * @param {number|undefined} contextId + * @param {number} timeout * @return {Promise<*>} */ - async _evaluateInContext(expression, contextId) { - // Use a higher than default timeout if the user hasn't specified a specific timeout. - // Otherwise, use whatever was requested. - const timeout = this._session.hasNextProtocolTimeout() ? - this._session.getNextProtocolTimeout() : - 60000; - + async _evaluateInContext(expression, contextId, timeout) { // `__lighthouseExecutionContextUniqueIdentifier` is only used by the FullPageScreenshot gatherer. // See `getNodeDetails` in page-functions. const uniqueExecutionContextIdentifier = contextId === undefined ? @@ -166,17 +161,22 @@ class ExecutionContext { * @return {Promise<*>} */ async evaluateAsync(expression, options = {}) { + // Use a higher than default timeout if the user hasn't specified a specific timeout. + // Otherwise, use whatever was requested. + const timeout = this._session.hasNextProtocolTimeout() ? + this._session.getNextProtocolTimeout() : + 60000; const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; try { // `await` is not redundant here because we want to `catch` the async errors - return await this._evaluateInContext(expression, contextId); + return await this._evaluateInContext(expression, contextId, timeout); } catch (err) { // If we were using isolation and the context disappeared on us, retry one more time. if (contextId && err.message.includes('Cannot find context')) { this.clearContextId(); const freshContextId = await this._getOrCreateIsolatedContextId(); - return this._evaluateInContext(expression, freshContextId); + return this._evaluateInContext(expression, freshContextId, timeout); } throw err; diff --git a/core/test/gather/driver/execution-context-test.js b/core/test/gather/driver/execution-context-test.js index b2b28fe2483f..d9dfd4640bf0 100644 --- a/core/test/gather/driver/execution-context-test.js +++ b/core/test/gather/driver/execution-context-test.js @@ -120,6 +120,27 @@ describe('.evaluateAsync', () => { await expect(evaluatePromise).rejects.toBeTruthy(); }); + it('uses the specific timeout given (isolation)', async () => { + const expectedTimeout = 5000; + const setNextProtocolTimeout = sessionMock.setNextProtocolTimeout = fnAny(); + sessionMock.hasNextProtocolTimeout = fnAny().mockReturnValue(true); + sessionMock.getNextProtocolTimeout = fnAny().mockReturnValue(expectedTimeout); + sessionMock.sendCommand + .mockResponse('Page.enable') + .mockResponse('Runtime.enable') + .mockResponse('Page.getFrameTree', {frameTree: {frame: {id: '1337'}}}) + .mockResponse('Page.createIsolatedWorld', {executionContextId: 1}); + + const evaluatePromise = makePromiseInspectable(executionContext.evaluateAsync('1 + 1', { + useIsolation: true, + })); + + await flushAllTimersAndMicrotasks(); + expect(setNextProtocolTimeout).toHaveBeenCalledWith(expectedTimeout); + expect(evaluatePromise).toBeDone(); + await expect(evaluatePromise).rejects.toBeTruthy(); + }); + it('evaluates an expression in isolation', async () => { sessionMock.sendCommand .mockResponse('Page.enable')