Skip to content

Commit

Permalink
proxyless: fix sporadic 'Invalid InterceptionId' error. (#7359)
Browse files Browse the repository at this point in the history
  • Loading branch information
miherlosev authored Nov 3, 2022
1 parent 3351b62 commit 74ff543
Show file tree
Hide file tree
Showing 8 changed files with 102 additions and 43 deletions.
3 changes: 0 additions & 3 deletions src/proxyless/request-pipeline/constants.ts
Original file line number Diff line number Diff line change
@@ -1,13 +1,10 @@
import CONTENT_TYPES from '../../assets/content-types';

const INVALID_INTERCEPTED_RESPONSE_ERROR_MSG = 'Invalid InterceptionId.';

const FAVICON_CONTENT_TYPE_HEADER = {
name: 'content-type',
value: CONTENT_TYPES.icon,
};

export {
INVALID_INTERCEPTED_RESPONSE_ERROR_MSG,
FAVICON_CONTENT_TYPE_HEADER,
};
14 changes: 12 additions & 2 deletions src/proxyless/request-pipeline/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import ProxylessPipelineContext from '../request-hooks/pipeline-context';
import { ProxylessSetupOptions } from '../../shared/types';
import DEFAULT_PROXYLESS_SETUP_OPTIONS from '../default-setup-options';
import getSpecialRequestHandler from './special-handlers';
import { safeContinueResponse } from './safe-api';


export default class ProxylessRequestPipeline {
Expand Down Expand Up @@ -95,6 +96,11 @@ export default class ProxylessRequestPipeline {
return continueResponseRequest;
}

private _shouldRedirectToErrorPage (event: RequestPausedEvent): boolean {
return event.resourceType === 'Document'
&& !this._isIframe(event.frameId);
}

private async _handleOtherRequests (event: RequestPausedEvent): Promise<void> {
requestPipelineOtherRequestLogger('%r', event);

Expand All @@ -120,15 +126,19 @@ export default class ProxylessRequestPipeline {
else {
const resourceInfo = await this._resourceInjector.getDocumentResourceInfo(event, this._client);

if (!resourceInfo.success)
if (resourceInfo.error) {
if (this._shouldRedirectToErrorPage(event))
await this._resourceInjector.redirectToErrorPage(this._client, resourceInfo.error, event.request.url);

return;
}

const modified = await this.requestHookEventProvider.onResponse(event, resourceInfo.body, this._client);

if (event.resourceType !== 'Document') {
const continueResponseRequest = this._createContinueResponseRequest(event, modified);

await this._client.Fetch.continueResponse(continueResponseRequest);
await safeContinueResponse(this._client, continueResponseRequest);
}
else {
await this._resourceInjector.processHTMLPageContent(
Expand Down
64 changes: 64 additions & 0 deletions src/proxyless/request-pipeline/safe-api.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
import { ProtocolApi } from 'chrome-remote-interface';
import { requestPipelineLogger } from '../../utils/debug-loggers';
import Protocol from 'devtools-protocol';
import RequestPausedEvent = Protocol.Fetch.RequestPausedEvent;
import FulfillRequestRequest = Protocol.Fetch.FulfillRequestRequest;
import ContinueResponseRequest = Protocol.Fetch.ContinueResponseRequest;
import { isRequestPausedEvent } from '../utils/cdp';

const INVALID_INTERCEPTED_RESPONSE_ERROR_MSG = 'Invalid InterceptionId.';

// In some cases (a request was aborted, any page that initiated the request doesn't exist, etc.)
// Chrome Debug Protocol doesn't allow to continue request pipeline
// and raises the "Invalid InterceptionId" error.
// We use the simplest way to fix it - omit such an error.

export async function safeContinueResponse (client: ProtocolApi, data: RequestPausedEvent | ContinueResponseRequest): Promise<void> {
const isPausedEvent = isRequestPausedEvent(data);

try {
const param = isPausedEvent
? { requestId: data.requestId }
: data;

await client.Fetch.continueResponse(param);
}
catch (err: any) {
if (err.message === INVALID_INTERCEPTED_RESPONSE_ERROR_MSG)
return;

const formatter = isPausedEvent ? '%r' : '%s';

requestPipelineLogger(`Fetch.continueResponse. Unhandled error %s during processing ${formatter}`, err, data);

throw err;
}
}

export async function safeFulfillRequest (client: ProtocolApi, fulfillInfo: FulfillRequestRequest): Promise<void> {
try {
await client.Fetch.fulfillRequest(fulfillInfo);
}
catch (err: any) {
if (err.message === INVALID_INTERCEPTED_RESPONSE_ERROR_MSG)
return;

requestPipelineLogger(`Fetch.fulfillRequest. Unhandled error %s during processing %s`, err, fulfillInfo.requestId);

throw err;
}
}

export async function safeContinueRequest (client: ProtocolApi, event: RequestPausedEvent): Promise<void> {
try {
await client.Fetch.continueRequest({ requestId: event.requestId });
}
catch (err: any) {
if (err.message === INVALID_INTERCEPTED_RESPONSE_ERROR_MSG)
return;

requestPipelineLogger(`Fetch.continueRequest. Unhandled error %s during processing %r`, err, event);

throw err;
}
}
28 changes: 7 additions & 21 deletions src/proxyless/request-pipeline/special-handlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,11 @@ import {
} from '../../utils/debug-loggers';
import { ProxylessSetupOptions } from '../../shared/types';
import { isRequest } from '../utils/cdp';
import { FAVICON_CONTENT_TYPE_HEADER, INVALID_INTERCEPTED_RESPONSE_ERROR_MSG } from './constants';
import { FAVICON_CONTENT_TYPE_HEADER } from './constants';
import { StatusCodes } from 'http-status-codes';
import loadAssets from '../../load-assets';
import { toBase64String } from '../utils/string';
import { safeContinueRequest, safeContinueResponse } from './safe-api';


const internalRequest = {
Expand Down Expand Up @@ -42,25 +43,10 @@ const serviceRequest = {
handler: async (event: RequestPausedEvent, client: ProtocolApi): Promise<void> => {
requestPipelineServiceRequestLogger('%r', event);

const { requestId } = event;

if (isRequest(event))
await client.Fetch.continueRequest({ requestId });
else {
// Hack: CDP doesn't allow to continue response for requests sent from the reloaded page.
// Such situation rarely occurs on 'heartbeat' or 'open-file-protocol' requests.
// We are using the simplest way to fix it - just omit such errors.
try {
await client.Fetch.continueResponse({ requestId });
}
catch (err: any) {
if (err.message === INVALID_INTERCEPTED_RESPONSE_ERROR_MSG)
return;

throw err;
}

}
await safeContinueRequest(client, event);
else
await safeContinueResponse(client, event);
},
} as RequestHandler;

Expand All @@ -74,7 +60,7 @@ const defaultFaviconRequest = {
requestPipelineLogger('%r', event);

if (isRequest(event))
await client.Fetch.continueRequest({ requestId: event.requestId });
await safeContinueRequest(client, event);
else {
if (event.responseStatusCode === StatusCodes.NOT_FOUND) { // eslint-disable-line no-lonely-if
const { favIcon } = loadAssets(options.developmentMode);
Expand All @@ -87,7 +73,7 @@ const defaultFaviconRequest = {
});
}
else
await client.Fetch.continueResponse({ requestId: event.requestId });
await safeContinueResponse(client, event);
}
},
} as RequestHandler;
Expand Down
27 changes: 12 additions & 15 deletions src/proxyless/resource-injector.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import {
stringifyHeaderValues,
toBase64String,
} from './utils/string';
import { safeFulfillRequest } from './request-pipeline/safe-api';


const CONTENT_SECURITY_POLICY_HEADER_NAMES = [
Expand Down Expand Up @@ -84,7 +85,7 @@ export default class ResourceInjector {
return stringifyHeaderValues(headers);
}

private async _handlePageError (client: ProtocolApi, err: Error, url: string): Promise<void> {
public async redirectToErrorPage (client: ProtocolApi, err: Error, url: string): Promise<void> {
const browserConnection = BrowserConnection.getById(this._browserId) as BrowserConnection;
const currentTestRun = browserConnection.getCurrentTestRun();

Expand All @@ -106,39 +107,35 @@ export default class ResourceInjector {

if (resourceType !== 'Document') {
return {
success: true,
body: null,
error: null,
body: null,
};
}

try {
if (responseErrorReason === 'NameNotResolved') {
const err = new Error(`Failed to find a DNS-record for the resource at "${event.request.url}"`);

await this._handlePageError(client, err, request.url);

return {
success: false,
body: null,
error: err,
body: null,
};
}

const responseObj = await client.Fetch.getResponseBody({ requestId });
const responseStr = getResponseAsString(responseObj);

return {
success: true,
body: Buffer.from(responseStr),
error: null,
body: Buffer.from(responseStr),
};
}
catch (err) {
resourceInjectorLogger('Failed to process request: %s', request.url);

await this._handlePageError(client, err as Error, request.url);

return {
success: false,
body: null,
error: err,
body: null,
};
}
}
Expand All @@ -165,7 +162,7 @@ export default class ResourceInjector {
else {
const updatedResponseStr = injectResources(fulfillRequestInfo.body as string, injectableResources);

await client.Fetch.fulfillRequest({
await safeFulfillRequest(client, {
requestId: fulfillRequestInfo.requestId,
responseCode: fulfillRequestInfo.responseCode || StatusCodes.OK,
responseHeaders: this._processResponseHeaders(fulfillRequestInfo.responseHeaders),
Expand All @@ -175,7 +172,7 @@ export default class ResourceInjector {
}

public async processNonProxiedContent (fulfillRequestInfo: FulfillRequestRequest, client: ProtocolApi): Promise<void> {
await client.Fetch.fulfillRequest({
await safeFulfillRequest(client, {
requestId: fulfillRequestInfo.requestId,
responseCode: fulfillRequestInfo.responseCode || StatusCodes.OK,
responseHeaders: this._processResponseHeaders(fulfillRequestInfo.responseHeaders),
Expand Down
2 changes: 1 addition & 1 deletion src/proxyless/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export interface SpecialServiceRoutes {
}

export interface DocumentResourceInfo {
success: boolean;
error: any;
body: Buffer | null;
}

Expand Down
4 changes: 4 additions & 0 deletions src/proxyless/utils/cdp.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,10 @@ export function isRequest (event: RequestPausedEvent): boolean {
return event.responseStatusCode === void 0;
}

export function isRequestPausedEvent (val: any): val is RequestPausedEvent {
return val && val.frameId && typeof val.request === 'object';
}

export function createRequestPausedEventForResponse (mockedResponse: IncomingMessageLike, requestEvent: RequestPausedEvent): RequestPausedEvent {
return Object.assign({}, requestEvent, {
responseStatusCode: mockedResponse.statusCode,
Expand Down
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
const { expect } = require('chai');
const { errorInEachBrowserContains } = require('../../../../assertion-helper.js');
const { skipDescribeInProxyless } = require('../../../../utils/skip-in');

// NOTE: we set selectorTimeout to a large value in some tests to wait for
// an iframe to load on the farm (it is fast locally but can take some time on the farm)
Expand Down Expand Up @@ -144,7 +145,7 @@ describe('[API] t.switchToIframe(), t.switchToMainWindow()', function () {
});
});

describe('Page errors handling', function () {
skipDescribeInProxyless('Page errors handling', function () {
it('Should fail if an error occurs in a same-domain iframe while an action is being executed', function () {
return runTests('./testcafe-fixtures/page-errors-test.js', 'Error in a same-domain iframe', DEFAULT_FAILED_RUN_OPTIONS)
.catch(function (errs) {
Expand Down

0 comments on commit 74ff543

Please sign in to comment.