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.

Sign up for GitHub

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

Jump to bottom

feat: React Native support with fetchOption and callOptions #653

Merged
merged 14 commits into from Nov 22, 2022
Merged

feat: React Native support with fetchOption and callOptions #653

Changes from 4 commits
Commits
Show all changes
14 commits
Select commit Hold shift + click to select a range
724d8fc
experimental changes
krpeacock Oct 24, 2022
a9e7d5d
added callOptions, and changed assignment logic of this._fetchOptions
wackyleo459 Nov 1, 2022
2ad9a20
Typo error. In case options.source is correctly implemented I've stil…
wackyleo459 Nov 1, 2022
2f07956
When source isn't provided in constructing the HttpAgent, this._fetch…
wackyleo459 Nov 8, 2022
3298d3c
experimental changes
krpeacock Oct 24, 2022
8b6b183
added callOptions, and changed assignment logic of this._fetchOptions
wackyleo459 Nov 1, 2022
425f0b1
Typo error. In case options.source is correctly implemented I've stil…
wackyleo459 Nov 1, 2022
1be96e0
When source isn't provided in constructing the HttpAgent, this._fetch…
wackyleo459 Nov 8, 2022
b1178c4
Merge branch 'kyle/fetchOptions' of https://github.com/dfinity/agent-…
wackyleo459 Nov 8, 2022
be9bf07
Migration Change from port 8000 to 4943 for new DFX 0.12.0
wackyleo459 Nov 10, 2022
4f659bd
Node 12 support deprecated, along with recent latest npm version 9.1.…
wackyleo459 Nov 10, 2022
ea15c11
Merge branch 'sueann/fetchOptions' into kyle/fetchOptions
wackyleo459 Nov 10, 2022
56071af
Unit test added for agent/http.test.ts
wackyleo459 Nov 16, 2022
b7c8e50
Test both call and query behavior
rvanasa Nov 17, 2022
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
16 changes: 15 additions & 1 deletion packages/agent/src/agent/http/index.ts
View file
Open in desktop
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