-
Notifications
You must be signed in to change notification settings - Fork 97
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: call and poll #941
Draft
krpeacock
wants to merge
8
commits into
main
Choose a base branch
from
kai/SDK-1828-call-and-poll
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
feat: call and poll #941
Changes from all commits
Commits
Show all changes
8 commits
Select commit
Hold shift + click to select a range
9d0ab85
feat: introduces callAndPoll utility returning the full undecoded cer…
krpeacock b4a90fb
Merge branch 'main' into kai/SDK-1828-call-and-poll
krpeacock d983a95
merging in trap changes
krpeacock a559435
chore: reverts change to trap test
krpeacock 059905a
feat: check for 200 response status, reveals additional details in ne…
krpeacock 5befbdb
wip
krpeacock 7b52f29
Merge branch 'main' into kai/SDK-1828-call-and-poll
krpeacock 8843eeb
wip
krpeacock File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,19 @@ | ||
import { HttpAgent, fromHex, callAndPoll } from '@dfinity/agent'; | ||
import { Principal } from '@dfinity/principal'; | ||
import { expect, describe, it, vi } from 'vitest'; | ||
describe('call and poll', () => { | ||
it('should handle call and poll', async () => { | ||
vi.useRealTimers(); | ||
|
||
const options = { | ||
canister_id: Principal.from('tnnnb-2yaaa-aaaab-qaiiq-cai'), | ||
method_name: 'inc_read', | ||
agent: await HttpAgent.create({ host: 'https://icp-api.io' }), | ||
arg: fromHex('4449444c0000'), | ||
}; | ||
|
||
const { certificate, contentMap } = await callAndPoll(options); | ||
expect(certificate instanceof ArrayBuffer).toBe(true); | ||
expect(contentMap).toMatchInlineSnapshot(); | ||
}); | ||
}); |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
import { Principal } from '@dfinity/principal'; | ||
import { | ||
Agent, | ||
Certificate, | ||
ContentMap, | ||
Expiry, | ||
PartialBy, | ||
bufFromBufLike, | ||
polling, | ||
v3ResponseBody, | ||
} from '..'; | ||
import { AgentCallError, AgentError } from '../errors'; | ||
import { isArrayBuffer } from 'util/types'; | ||
import { DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS } from '../constants'; | ||
|
||
export type CallAndPollOptions = PartialBy< | ||
Omit<ContentMap, 'sender' | 'request_type'>, | ||
'ingress_expiry' | ||
> & { | ||
agent: Agent; | ||
}; | ||
|
||
/** | ||
* Call a canister using the v3 api and either return the response or fall back to polling | ||
* @param options - The options to use when calling the canister | ||
* @param options.canister_id - The canister id to call | ||
* @param options.method_name - The method name to call | ||
* @param options.agent - The agent to use to make the call | ||
* @param options.arg - The argument to pass to the canister | ||
* @returns The certificate response from the canister (which includes the reply) | ||
*/ | ||
export async function callAndPoll(options: CallAndPollOptions): Promise<{ | ||
certificate: ArrayBuffer; | ||
contentMap: ContentMap; | ||
}> { | ||
assertContentMap(options); | ||
const { canister_id, method_name, agent, arg } = options; | ||
const cid = Principal.from(options.canister_id); | ||
|
||
const { defaultStrategy } = polling.strategy; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Would it be useful for polling strategy to be an option? |
||
|
||
if (agent.rootKey == null) throw new Error('Agent root key not initialized before making call'); | ||
|
||
const ingress_expiry = | ||
options.ingress_expiry ?? new Expiry(DEFAULT_INGRESS_EXPIRY_DELTA_IN_MSECS); | ||
|
||
const contentMap: ContentMap = { | ||
canister_id: Principal.from(canister_id), | ||
request_type: 'call', | ||
method_name: method_name, | ||
arg, | ||
sender: await agent.getPrincipal(), | ||
ingress_expiry, | ||
}; | ||
const { requestId, response } = await agent.call(cid, { | ||
methodName: method_name, | ||
arg, | ||
effectiveCanisterId: cid, | ||
ingressExpiry: ingress_expiry, | ||
}); | ||
|
||
if (response.status === 200) { | ||
if ('body' in response) { | ||
// Ensure the response body is a v3 response | ||
assertV3ResponseBody(response.body); | ||
|
||
const cert = response.body.certificate; | ||
// Create certificate to validate the responses | ||
const certificate = await Certificate.create({ | ||
certificate: bufFromBufLike(cert), | ||
rootKey: agent.rootKey, | ||
canisterId: Principal.from(canister_id), | ||
}); | ||
|
||
return { | ||
certificate: certificate.rawCert, | ||
contentMap, | ||
}; | ||
} else { | ||
throw new AgentCallError( | ||
'unexpected call error: no certificate in response', | ||
response, | ||
requestId, | ||
); | ||
} | ||
} | ||
// Fall back to polling if we recieve an Accepted response code | ||
else if (response.status === 202) { | ||
const pollStrategy = defaultStrategy(); | ||
// Contains the certificate and the reply from the boundary node | ||
const response = await polling.pollForResponse(agent, cid, requestId, pollStrategy); | ||
return { | ||
certificate: response.certificate.rawCert, | ||
contentMap, | ||
}; | ||
} else { | ||
console.error('The network returned a response but the result could not be determined.', { | ||
response, | ||
requestId, | ||
}); | ||
throw new AgentError('unexpected call error: no certificate in response'); | ||
} | ||
} | ||
|
||
function assertContentMap(contentMap: unknown): asserts contentMap is ContentMap { | ||
if (!contentMap || typeof contentMap !== 'object') { | ||
throw new AgentError('unexpected call error: no contentMap provided for call'); | ||
} | ||
if (!('canister_id' in contentMap)) { | ||
throw new AgentError('unexpected call error: no canister_id provided for call'); | ||
} | ||
if (!('method_name' in contentMap)) { | ||
throw new AgentError('unexpected call error: no method_name provided for call'); | ||
} | ||
if (!('arg' in contentMap)) { | ||
throw new AgentError('unexpected call error: no arg provided for call'); | ||
} | ||
} | ||
|
||
function assertV3ResponseBody(body: unknown): asserts body is v3ResponseBody { | ||
if (!body || typeof body !== 'object') { | ||
throw new AgentError('unexpected call error: no body in response'); | ||
} | ||
if (!('certificate' in body)) { | ||
throw new AgentError('unexpected call error: no certificate in response'); | ||
} | ||
if (body['certificate'] === undefined || body['certificate'] === null) { | ||
throw new AgentError('unexpected call error: certificate is not an ArrayBuffer'); | ||
} | ||
try { | ||
const cert = bufFromBufLike(body['certificate'] as ArrayBufferLike); | ||
if (!isArrayBuffer(cert)) { | ||
throw new AgentError('unexpected call error: certificate is not an ArrayBuffer'); | ||
} | ||
} catch (error) { | ||
throw new AgentError('unexpected call error: while presenting certificate: ' + error); | ||
} | ||
} |
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are you going to enhance the tests? e.g. asserting that the certificate return is the expected value or asserting that the function correctly throws if there was an error?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, this should have been marked as a draft. I wanted feedback on the inputs and return values before proceeding. If you have real-world test cases this feature will be used for, those would be great to add as tests as well, beyond the basic cases!