Skip to content
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

feat: React Native support with fetchOption and callOptions #653

Merged
merged 14 commits into from Nov 22, 2022
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 2 additions & 2 deletions .github/workflows/e2e-tests.yml
Expand Up @@ -45,12 +45,12 @@ jobs:
- run: npm run e2e --workspaces --if-present
env:
CI: true
REPLICA_PORT: 8000
REPLICA_PORT: 4943

aggregate:
name: e2e:required
if: ${{ always() }}
needs: [ test ]
needs: [test]
runs-on: ubuntu-latest
steps:
- name: check e2e test result
Expand Down
4 changes: 2 additions & 2 deletions .github/workflows/mitm.yml
Expand Up @@ -51,7 +51,7 @@ jobs:
id: mitmdump
run: |
set -ex
mitmdump -p 8888 --mode reverse:http://127.0.0.1:8000 \
mitmdump -p 8888 --mode reverse:http://127.0.0.1:4943 \
--modify-headers '/~s/Transfer-Encoding/' \
--modify-body '/~s/Hello/Hullo' \
&
Expand All @@ -67,7 +67,7 @@ jobs:
aggregate:
name: mitm:required
if: ${{ always() }}
needs: [ test ]
needs: [test]
runs-on: ubuntu-latest
steps:
- name: check e2e test result
Expand Down
1 change: 0 additions & 1 deletion .github/workflows/unit-tests.yml
Expand Up @@ -19,7 +19,6 @@ jobs:
spec:
- '0.16.1'
node:
- 12
- 14
- 16

Expand Down
2 changes: 1 addition & 1 deletion e2e/browser/.proxyrc
@@ -1,5 +1,5 @@
{
"/api": {
"target": "http://localhost:8000/",
"target": "http://localhost:4943/",
}
}
62 changes: 52 additions & 10 deletions packages/agent/src/agent/http/http.test.ts
Expand Up @@ -22,6 +22,8 @@ const { window } = new JSDOM(`<!DOCTYPE html><p>Hello world</p>`);
window.fetch = global.fetch;
(global as any).window = window;

const HTTP_AGENT_HOST = 'http://localhost:4943';

const DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS = 5 * 60 * 1000;
const REPLICA_PERMITTED_DRIFT_MILLISECONDS = 60 * 1000;
const NANOSECONDS_PER_MILLISECONDS = 1_000_000;
Expand Down Expand Up @@ -313,26 +315,26 @@ test('use anonymous principal if unspecified', async () => {

describe('getDefaultFetch', () => {
it("should use fetch from window if it's available", async () => {
const generateAgent = () => new HttpAgent({ host: 'localhost:8000' });
const generateAgent = () => new HttpAgent({ host: HTTP_AGENT_HOST });
expect(generateAgent).not.toThrowError();
});
it('should throw an error if fetch is not available on the window object', async () => {
delete (window as any).fetch;
const generateAgent = () => new HttpAgent({ host: 'localhost:8000' });
const generateAgent = () => new HttpAgent({ host: HTTP_AGENT_HOST });

expect(generateAgent).toThrowError('Fetch implementation was not available');
});
it('should throw error for defaultFetch with no window or global fetch', () => {
delete (global as any).window;
delete (global as any).fetch;
const generateAgent = () => new HttpAgent({ host: 'localhost:8000' });
const generateAgent = () => new HttpAgent({ host: HTTP_AGENT_HOST });

expect(generateAgent).toThrowError('Fetch implementation was not available');
});
it('should fall back to global.fetch if window is not available', () => {
delete (global as any).window;
global.fetch = originalFetch;
const generateAgent = () => new HttpAgent({ host: 'localhost:8000' });
const generateAgent = () => new HttpAgent({ host: HTTP_AGENT_HOST });

expect(generateAgent).not.toThrowError();
});
Expand Down Expand Up @@ -494,7 +496,7 @@ describe('retry failures', () => {
statusText: 'Internal Server Error',
}),
);
const agent = new HttpAgent({ host: 'http://localhost:8000', fetch: mockFetch, retryTimes: 0 });
const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch, retryTimes: 0 });
expect(
agent.call(Principal.managementCanister(), {
methodName: 'test',
Expand All @@ -510,7 +512,7 @@ describe('retry failures', () => {
});
});

const agent = new HttpAgent({ host: 'http://localhost:8000', fetch: mockFetch });
const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch });
try {
expect(
agent.call(Principal.managementCanister(), {
Expand Down Expand Up @@ -540,7 +542,7 @@ describe('retry failures', () => {
}
});

const agent = new HttpAgent({ host: 'http://localhost:8000', fetch: mockFetch });
const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch });
const result = await agent.call(Principal.managementCanister(), {
methodName: 'test',
arg: new Uint8Array().buffer,
Expand All @@ -556,7 +558,7 @@ test('should change nothing if time is within 30 seconds of replica', async () =
// jest.setSystemTime(systemTime);
const mockFetch = jest.fn();

const agent = new HttpAgent({ host: 'http://localhost:8000', fetch: mockFetch });
const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch });

await agent.syncTime();

Expand Down Expand Up @@ -590,7 +592,7 @@ test('should adjust the Expiry if the clock is more than 30 seconds behind', asy
await import('../../canisterStatus');
const { HttpAgent } = await import('../index');

const agent = new HttpAgent({ host: 'http://localhost:8000', fetch: mockFetch });
const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch });

await agent.syncTime();

Expand Down Expand Up @@ -633,7 +635,7 @@ test('should adjust the Expiry if the clock is more than 30 seconds ahead', asyn
await import('../../canisterStatus');
const { HttpAgent } = await import('../index');

const agent = new HttpAgent({ host: 'http://localhost:8000', fetch: mockFetch });
const agent = new HttpAgent({ host: HTTP_AGENT_HOST, fetch: mockFetch });

await agent.syncTime();

Expand All @@ -657,3 +659,43 @@ test('should adjust the Expiry if the clock is more than 30 seconds ahead', asyn
expect(delay).toBe(-1 * DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS);
jest.resetModules();
});

test('should fetch with given call options and fetch options', async () => {
const mockFetch: jest.Mock = jest.fn(() => {
const body = cbor.encode({});
return Promise.resolve(
new Response(body, {
status: 200,
}),
);
});

const canisterId: Principal = Principal.fromText('2chl6-4hpzw-vqaaa-aaaaa-c');
const httpAgent = new HttpAgent({
fetch: mockFetch,
host: 'http://localhost',
callOptions: {
reactNative: { textStreaming: true },
},
fetchOptions: {
reactNative: {
__nativeResponseType: 'base64',
},
},
});

await httpAgent.call(canisterId, {
methodName: 'greet',
arg: new Uint8Array([]),
});

await httpAgent.query(canisterId, {
methodName: 'greet',
arg: new Uint8Array([]),
});

const { calls } = mockFetch.mock;

expect(calls[0][1].reactNative).toStrictEqual({ textStreaming: true });
expect(calls[1][1].reactNative.__nativeResponseType).toBe('base64');
});
16 changes: 15 additions & 1 deletion packages/agent/src/agent/http/index.ts
Expand Up @@ -72,6 +72,13 @@ export interface HttpAgentOptions {
// A surrogate to the global fetch function. Useful for testing.
fetch?: typeof fetch;

// Additional options to pass along to fetch. Will not override fields that
// the agent already needs to set
fetchOptions?: Record<string, unknown>;

// Additional options to pass along to fetch for the call API.
callOptions?: Record<string, unknown>;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might eventually make sense to include one or two unit tests to ensure that these options can be used and overridden as expected. The implementation looks good, so this would just be to minimize the chance that someone doesn't accidentally break the expected behavior a few years from now (since a regression might cause some very subtle bugs that could be difficult to identify otherwise).

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've attempted to create a minor unit test here.
There is a repeated error that shows (node:2473) V8: /home/runner/work/agent-js/agent-js/node_modules/borc/src/decoder.asm.js:3 Linking failure in asm.js: Unexpected stdlib member.
Not sure but this might be due to issue here of jest globals and node globals differing. jestjs/jest#2549.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, could be something like that. What Node.js version are you using?

The unit test works as expected on my laptop (equivalent to the CI output).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I pushed some fixes for the unit test to hopefully save you a bit of time. Feel free to make any changes or improvements as you see fit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks so much! I was using node version 16.17.0. (I'm still not sure how to get the unit test to work locally.) But I'm glad this fix works!


// The host to use for the client. By default, uses the same host as
// the current page.
host?: string;
Expand Down Expand Up @@ -162,6 +169,8 @@ export class HttpAgent implements Agent {
private readonly _pipeline: HttpAgentRequestTransformFn[] = [];
private _identity: Promise<Identity> | null;
private readonly _fetch: typeof fetch;
private readonly _fetchOptions?: Record<string, unknown>;
private readonly _callOptions?: Record<string, unknown>;
private _timeDiffMsecs = 0;
private readonly _host: URL;
private readonly _credentials: string | undefined;
Expand All @@ -181,6 +190,8 @@ export class HttpAgent implements Agent {
this._credentials = options.source._credentials;
} else {
this._fetch = options.fetch || getDefaultFetch() || fetch.bind(global);
this._fetchOptions = options.fetchOptions;
this._callOptions = options.callOptions;
}
if (options.host !== undefined) {
if (!options.host.match(/^[a-z]+:/) && typeof window !== 'undefined') {
Expand Down Expand Up @@ -301,6 +312,7 @@ export class HttpAgent implements Agent {

const request = this._requestAndRetry(() =>
this._fetch('' + new URL(`/api/v2/canister/${ecid.toText()}/call`, this._host), {
...this._callOptions,
...transformedRequest.request,
body,
}),
Expand Down Expand Up @@ -386,6 +398,7 @@ export class HttpAgent implements Agent {
const body = cbor.encode(transformedRequest.body);
const response = await this._requestAndRetry(() =>
this._fetch('' + new URL(`/api/v2/canister/${canister.toText()}/query`, this._host), {
...this._fetchOptions,
...transformedRequest.request,
body,
}),
Expand Down Expand Up @@ -445,6 +458,7 @@ export class HttpAgent implements Agent {
const response = await this._fetch(
'' + new URL(`/api/v2/canister/${canister}/read_state`, this._host),
{
...this._fetchOptions,
...transformedRequest.request,
body,
},
Expand Down Expand Up @@ -497,7 +511,7 @@ export class HttpAgent implements Agent {
: {};

const response = await this._requestAndRetry(() =>
this._fetch('' + new URL(`/api/v2/status`, this._host), { headers }),
this._fetch('' + new URL(`/api/v2/status`, this._host), { headers, ...this._fetchOptions }),
);

return cbor.decode(await response.arrayBuffer());
Expand Down
2 changes: 1 addition & 1 deletion packages/agent/src/canisterStatus/index.test.ts
Expand Up @@ -46,7 +46,7 @@ const getRealStatus = async () => {
),
)) as unknown as Identity;

const agent = new HttpAgent({ host: 'http://127.0.0.1:8000', fetch, identity });
const agent = new HttpAgent({ host: 'http://127.0.0.1:4943', fetch, identity });
await agent.fetchRootKey();
const canisterBuffer = new DataView(testPrincipal.toUint8Array().buffer).buffer;
canisterBuffer;
Expand Down
2 changes: 1 addition & 1 deletion packages/auth-client/src/index.test.ts
Expand Up @@ -85,7 +85,7 @@ describe('Auth Client', () => {
(window as any).location = {
reload: jest.fn(),
fetch,
toString: jest.fn(() => 'http://localhost:8000'),
toString: jest.fn(() => 'http://localhost:4943'),
};

const identity = Ed25519KeyIdentity.generate();
Expand Down