Skip to content

Commit

Permalink
fix: Make ResponseWrapper an instance of Response (#2889)
Browse files Browse the repository at this point in the history
Overwrite `[Symbol.hasInstance]` on the exposed `Response` class
slightly to make `ResponseWrapper` pass `instanceof` checks. This
prevents `fetch` responses from failing when using WASM bindgen for
instance.

Fixes #2891
  • Loading branch information
FrederikBolding authored Nov 19, 2024
1 parent 2d6575e commit 96a29cb
Show file tree
Hide file tree
Showing 5 changed files with 84 additions and 10 deletions.
8 changes: 4 additions & 4 deletions packages/snaps-execution-environments/coverage.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"branches": 80,
"functions": 90.06,
"lines": 90.75,
"statements": 90.12
"branches": 80.27,
"functions": 90.13,
"lines": 90.77,
"statements": 90.15
}
Original file line number Diff line number Diff line change
Expand Up @@ -999,6 +999,57 @@ describe('BaseSnapExecutor', () => {
});
});

it('uses a response for fetch that works with instanceof', async () => {
const CODE = `
module.exports.onRpcRequest = () => fetch('https://metamask.io').then(res => res instanceof Response);
`;

const fetchSpy = spy(globalThis, 'fetch');

fetchSpy.mockImplementation(async () => {
return new Response('foo');
});

const executor = new TestSnapExecutor();
await executor.executeSnap(1, MOCK_SNAP_ID, CODE, ['fetch', 'Response']);

expect(await executor.readCommand()).toStrictEqual({
jsonrpc: '2.0',
id: 1,
result: 'OK',
});

await executor.writeCommand({
jsonrpc: '2.0',
id: 2,
method: 'snapRpc',
params: [
MOCK_SNAP_ID,
HandlerType.OnRpcRequest,
MOCK_ORIGIN,
{ jsonrpc: '2.0', method: '', params: [] },
],
});

expect(await executor.readCommand()).toStrictEqual({
jsonrpc: '2.0',
method: 'OutboundRequest',
params: { source: 'fetch' },
});

expect(await executor.readCommand()).toStrictEqual({
jsonrpc: '2.0',
method: 'OutboundResponse',
params: { source: 'fetch' },
});

expect(await executor.readCommand()).toStrictEqual({
id: 2,
jsonrpc: '2.0',
result: true,
});
});

it('notifies execution service of out of band errors via unhandledrejection', async () => {
const CODE = `
module.exports.onRpcRequest = async () => 'foo';
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import CryptoEndowment from './crypto';
import date from './date';
import interval from './interval';
import math from './math';
import network from './network';
import network, { ResponseWrapper } from './network';
import timeout from './timeout';

// @ts-expect-error - `globalThis.process` is not optional.
Expand Down Expand Up @@ -218,6 +218,19 @@ describe('endowments', () => {
endowments: { fetchAttenuated },
factory: () => undefined,
},
ResponseWrapper: {
endowments: {
Response: ResponseHardened,
ResponseWrapper: harden(ResponseWrapper),
},
factory: () =>
new ResponseWrapper(
new Response(),
{ lastTeardown: 0 },
async () => undefined,
async () => undefined,
),
},
};

Object.entries(TEST_ENDOWMENTS).forEach(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import fetchMock from 'jest-fetch-mock';

import network from './network';
import network, { ResponseWrapper } from './network';

describe('Network endowments', () => {
beforeAll(() => {
Expand Down Expand Up @@ -180,14 +180,15 @@ describe('Network endowments', () => {
const RESULT = 'OK';
fetchMock.mockOnce(async () => Promise.resolve(RESULT));

const { fetch } = network.factory(factoryOptions);
const { fetch, Response } = network.factory(factoryOptions);
const result = await fetch('foo.com');

expect(result.bodyUsed).toBe(false);
const clonedResult = result.clone();
expect(clonedResult).toBeDefined();
expect(await clonedResult.text()).toBe(RESULT);
expect(clonedResult).not.toBeInstanceOf(Response);
expect(clonedResult).toBeInstanceOf(Response);
expect(clonedResult).toBeInstanceOf(ResponseWrapper);
});

it('should return when json is called', async () => {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import type { EndowmentFactoryOptions } from './commonEndowmentFactory';
* This class wraps a Response object.
* That way, a teardown process can stop any processes left.
*/
class ResponseWrapper implements Response {
export class ResponseWrapper implements Response {
readonly #teardownRef: { lastTeardown: number };

#ogResponse: Response;
Expand Down Expand Up @@ -145,6 +145,15 @@ class ResponseWrapper implements Response {
}
}

// We redefine the global Response class to overwrite [Symbol.hasInstance].
// This fixes problems where the response from `fetch` would not pass
// instance of checks, leading to failures in WASM bindgen.
class AlteredResponse extends Response {
static [Symbol.hasInstance](instance: unknown) {
return instance instanceof Response || instance instanceof ResponseWrapper;
}
}

/**
* Create a network endowment, consisting of a `fetch` function.
* This allows us to provide a teardown function, so that we can cancel
Expand Down Expand Up @@ -297,7 +306,7 @@ const createNetwork = ({ notify }: EndowmentFactoryOptions = {}) => {
// These endowments are not (and should never be) available by default.
Request: harden(Request),
Headers: harden(Headers),
Response: harden(Response),
Response: harden(AlteredResponse),
teardownFunction,
};
};
Expand Down

0 comments on commit 96a29cb

Please sign in to comment.