From 7a9d17030b7596594681f0dcd0a406a3659409cb Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Mon, 22 Jul 2024 16:24:15 -0700 Subject: [PATCH 1/5] feat: improved assertion options for agent errors --- docs/CHANGELOG.md | 4 ++ e2e/node/integration/actor.test.ts | 4 +- packages/agent/src/actor.test.ts | 17 +++--- packages/agent/src/actor.ts | 73 +------------------------ packages/agent/src/errors.test.ts | 88 ++++++++++++++++++++++++++++++ packages/agent/src/errors.ts | 78 ++++++++++++++++++++++++++ 6 files changed, 183 insertions(+), 81 deletions(-) create mode 100644 packages/agent/src/errors.test.ts diff --git a/docs/CHANGELOG.md b/docs/CHANGELOG.md index 48f2a12fc..e4b7ccf50 100644 --- a/docs/CHANGELOG.md +++ b/docs/CHANGELOG.md @@ -2,6 +2,10 @@ ## [Unreleased] +## Added + +- feat: improved assertion options for agent errors using `prototype`, `name`, and `instanceof` + ## [2.0.0] - 2024-07-16 ### Changed diff --git a/e2e/node/integration/actor.test.ts b/e2e/node/integration/actor.test.ts index 49b48cf4c..855dce287 100644 --- a/e2e/node/integration/actor.test.ts +++ b/e2e/node/integration/actor.test.ts @@ -14,9 +14,9 @@ test("Legacy Agent interface should be accepted by Actor's createActor", async ( ); // Verify that update calls work - await actor.write(8n); //? + await actor.write(8n); // Verify that query calls work - const count = await actor.read(); //? + const count = await actor.read(); expect(count).toBe(8n); }, 15_000); // TODO: tests for rejected, unknown time out diff --git a/packages/agent/src/actor.test.ts b/packages/agent/src/actor.test.ts index 8f3ef481e..4a7906741 100644 --- a/packages/agent/src/actor.test.ts +++ b/packages/agent/src/actor.test.ts @@ -7,6 +7,7 @@ import * as cbor from './cbor'; import { requestIdOf } from './request_id'; import * as pollingImport from './polling'; import { ActorConfig } from './actor'; +import { UpdateCallRejectedError } from './errors'; const importActor = async (mockUpdatePolling?: () => void) => { jest.dontMock('./polling'); @@ -27,7 +28,7 @@ afterEach(() => { describe('makeActor', () => { // TODO: update tests to be compatible with changes to Certificate it.skip('should encode calls', async () => { - const { Actor, UpdateCallRejectedError } = await importActor(); + const { Actor } = await importActor(); const actorInterface = () => { return IDL.Service({ greet: IDL.Func([IDL.Text], [IDL.Text]), @@ -45,7 +46,7 @@ describe('makeActor', () => { }), ); }) - .mockImplementationOnce((resource, init) => { + .mockImplementationOnce(() => { const body = cbor.encode({ status: 'received' }); return Promise.resolve( new Response(body, { @@ -53,7 +54,7 @@ describe('makeActor', () => { }), ); }) - .mockImplementationOnce((resource, init) => { + .mockImplementationOnce(() => { const body = cbor.encode({ status: 'processing' }); return Promise.resolve( new Response(body, { @@ -61,7 +62,7 @@ describe('makeActor', () => { }), ); }) - .mockImplementationOnce((resource, init) => { + .mockImplementationOnce(() => { const body = cbor.encode({ status: 'replied', reply: { @@ -74,7 +75,7 @@ describe('makeActor', () => { }), ); }) - .mockImplementationOnce((resource, init) => { + .mockImplementationOnce(() => { // IC-1462 update call error const body = cbor.encode({ error_code: 'IC0503', @@ -292,7 +293,7 @@ describe('makeActor', () => { expect(reply).toEqual(canisterDecodedReturnValue); expect(replyUpdate).toEqual(canisterDecodedReturnValue); expect(replyWithHttpDetails.result).toEqual(canisterDecodedReturnValue); - replyWithHttpDetails.httpDetails['requestDetails']; //? + replyWithHttpDetails.httpDetails['requestDetails']; expect(replyWithHttpDetails.httpDetails).toMatchInlineSnapshot(` { "headers": [], @@ -330,7 +331,7 @@ describe('makeActor', () => { `); expect(replyUpdateWithHttpDetails.result).toEqual(canisterDecodedReturnValue); - replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); //? + replyUpdateWithHttpDetails.httpDetails['requestDetails']['nonce'] = new Uint8Array(); expect(replyUpdateWithHttpDetails.httpDetails).toMatchSnapshot(); }); @@ -364,7 +365,7 @@ describe('makeActor', () => { }); }; const { Actor } = await importActor(); - const config = { agent: httpAgent } as any as ActorConfig; + const config = { agent: httpAgent } as unknown as ActorConfig; expect(() => Actor.createActor(actorInterface, config)).toThrowError( 'Canister ID is required, but received undefined instead. If you are using automatically generated declarations, this may be because your application is not setting the canister ID in process.env correctly.', ); diff --git a/packages/agent/src/actor.ts b/packages/agent/src/actor.ts index 88fe7f9d1..4b9ca446d 100644 --- a/packages/agent/src/actor.ts +++ b/packages/agent/src/actor.ts @@ -1,82 +1,13 @@ import { Buffer } from 'buffer/'; -import { - Agent, - getDefaultAgent, - HttpDetailsResponse, - QueryResponseRejected, - QueryResponseStatus, - ReplicaRejectCode, - SubmitResponse, -} from './agent'; -import { AgentError } from './errors'; +import { Agent, getDefaultAgent, HttpDetailsResponse, QueryResponseStatus } from './agent'; +import { AgentError, QueryCallRejectedError, UpdateCallRejectedError } from './errors'; import { IDL } from '@dfinity/candid'; import { pollForResponse, PollStrategyFactory, strategy } from './polling'; import { Principal } from '@dfinity/principal'; -import { RequestId } from './request_id'; -import { toHex } from './utils/buffer'; import { Certificate, CreateCertificateOptions } from './certificate'; import managementCanisterIdl from './canisters/management_idl'; import _SERVICE, { canister_install_mode, canister_settings } from './canisters/management_service'; -export class ActorCallError extends AgentError { - constructor( - public readonly canisterId: Principal, - public readonly methodName: string, - public readonly type: 'query' | 'update', - public readonly props: Record, - ) { - super( - [ - `Call failed:`, - ` Canister: ${canisterId.toText()}`, - ` Method: ${methodName} (${type})`, - ...Object.getOwnPropertyNames(props).map(n => ` "${n}": ${JSON.stringify(props[n])}`), - ].join('\n'), - ); - } -} - -export class QueryCallRejectedError extends ActorCallError { - constructor( - canisterId: Principal, - methodName: string, - public readonly result: QueryResponseRejected, - ) { - super(canisterId, methodName, 'query', { - Status: result.status, - Code: ReplicaRejectCode[result.reject_code] ?? `Unknown Code "${result.reject_code}"`, - Message: result.reject_message, - }); - } -} - -export class UpdateCallRejectedError extends ActorCallError { - constructor( - canisterId: Principal, - methodName: string, - public readonly requestId: RequestId, - public readonly response: SubmitResponse['response'], - ) { - super(canisterId, methodName, 'update', { - 'Request ID': toHex(requestId), - ...(response.body - ? { - ...(response.body.error_code - ? { - 'Error code': response.body.error_code, - } - : {}), - 'Reject code': String(response.body.reject_code), - 'Reject message': response.body.reject_message, - } - : { - 'HTTP status code': response.status.toString(), - 'HTTP status text': response.statusText, - }), - }); - } -} - /** * Configuration to make calls to the Replica. */ diff --git a/packages/agent/src/errors.test.ts b/packages/agent/src/errors.test.ts new file mode 100644 index 000000000..5e4fe2d5d --- /dev/null +++ b/packages/agent/src/errors.test.ts @@ -0,0 +1,88 @@ +/* eslint-disable no-prototype-builtins */ +import { QueryResponseStatus, SubmitResponse } from './agent'; +import { + ActorCallError, + AgentError, + QueryCallRejectedError, + UpdateCallRejectedError, +} from './errors'; +import { RequestId } from './request_id'; + +test('AgentError', () => { + const error = new AgentError('message'); + expect(error.message).toBe('message'); + expect(error.name).toBe('AgentError'); + expect(error instanceof Error).toBe(true); + expect(error instanceof AgentError).toBe(true); + expect(error instanceof ActorCallError).toBe(false); + expect(AgentError.prototype.isPrototypeOf(error)).toBe(true); +}); + +test('ActorCallError', () => { + const error = new ActorCallError('rrkah-fqaaa-aaaaa-aaaaq-cai', 'methodName', 'query', { + props: 'props', + }); + expect(error.message).toBe(`Call failed: + Canister: rrkah-fqaaa-aaaaa-aaaaq-cai + Method: methodName (query) + "props": "props"`); + expect(error.name).toBe('ActorCallError'); + expect(error instanceof Error).toBe(true); + expect(error instanceof AgentError).toBe(true); + expect(error instanceof ActorCallError).toBe(true); + expect(ActorCallError.prototype.isPrototypeOf(error)).toBe(true); +}); + +test('QueryCallRejectedError', () => { + const error = new QueryCallRejectedError('rrkah-fqaaa-aaaaa-aaaaq-cai', 'methodName', { + status: QueryResponseStatus.Rejected, + reject_code: 1, + reject_message: 'reject_message', + error_code: 'error_code', + }); + expect(error.message).toBe(`Call failed: + Canister: rrkah-fqaaa-aaaaa-aaaaq-cai + Method: methodName (query) + "Status": "rejected" + "Code": "SysFatal" + "Message": "reject_message"`); + expect(error.name).toBe('QueryCallRejectedError'); + expect(error instanceof Error).toBe(true); + expect(error instanceof AgentError).toBe(true); + expect(error instanceof ActorCallError).toBe(true); + expect(error instanceof QueryCallRejectedError).toBe(true); + expect(QueryCallRejectedError.prototype.isPrototypeOf(error)).toBe(true); +}); + +test('UpdateCallRejectedError', () => { + const response: SubmitResponse['response'] = { + ok: false, + status: 400, + statusText: 'rejected', + body: { + error_code: 'error_code', + reject_code: 1, + reject_message: 'reject_message', + }, + headers: [], + }; + const error = new UpdateCallRejectedError( + 'rrkah-fqaaa-aaaaa-aaaaq-cai', + 'methodName', + new ArrayBuffer(1) as RequestId, + response, + ); + expect(error.message).toBe(`Call failed: + Canister: rrkah-fqaaa-aaaaa-aaaaq-cai + Method: methodName (update) + "Request ID": "00" + "Error code": "error_code" + "Reject code": "1" + "Reject message": "reject_message"`); + expect(error.name).toBe('UpdateCallRejectedError'); + expect(error instanceof Error).toBe(true); + expect(error instanceof AgentError).toBe(true); + expect(error instanceof ActorCallError).toBe(true); + expect(error instanceof UpdateCallRejectedError).toBe(true); + expect(UpdateCallRejectedError.prototype.isPrototypeOf(error)).toBe(true); +}); diff --git a/packages/agent/src/errors.ts b/packages/agent/src/errors.ts index d2e759f93..c7aec2b07 100644 --- a/packages/agent/src/errors.ts +++ b/packages/agent/src/errors.ts @@ -1,3 +1,8 @@ +import { Principal } from '@dfinity/principal'; +import { QueryResponseRejected, ReplicaRejectCode, SubmitResponse } from './agent/api'; +import { RequestId } from './request_id'; +import { toHex } from './utils/buffer'; + /** * An error that happens in the Agent. This is the root of all errors and should be used * everywhere in the Agent code (this package). @@ -5,8 +10,81 @@ * @todo https://github.com/dfinity/agent-js/issues/420 */ export class AgentError extends Error { + public name = 'AgentError'; + public __proto__ = AgentError.prototype; constructor(public readonly message: string) { super(message); Object.setPrototypeOf(this, AgentError.prototype); } } + +export class ActorCallError extends AgentError { + public name = 'ActorCallError'; + public __proto__ = ActorCallError.prototype; + constructor( + public readonly canisterId: Principal | string, + public readonly methodName: string, + public readonly type: 'query' | 'update', + public readonly props: Record, + ) { + const cid = Principal.from(canisterId); + super( + [ + `Call failed:`, + ` Canister: ${cid.toText()}`, + ` Method: ${methodName} (${type})`, + ...Object.getOwnPropertyNames(props).map(n => ` "${n}": ${JSON.stringify(props[n])}`), + ].join('\n'), + ); + Object.setPrototypeOf(this, ActorCallError.prototype); + } +} + +export class QueryCallRejectedError extends ActorCallError { + public name = 'QueryCallRejectedError'; + public __proto__ = QueryCallRejectedError.prototype; + constructor( + canisterId: Principal | string, + methodName: string, + public readonly result: QueryResponseRejected, + ) { + const cid = Principal.from(canisterId); + super(cid, methodName, 'query', { + Status: result.status, + Code: ReplicaRejectCode[result.reject_code] ?? `Unknown Code "${result.reject_code}"`, + Message: result.reject_message, + }); + Object.setPrototypeOf(this, QueryCallRejectedError.prototype); + } +} + +export class UpdateCallRejectedError extends ActorCallError { + public name = 'UpdateCallRejectedError'; + public __proto__ = UpdateCallRejectedError.prototype; + constructor( + canisterId: Principal | string, + methodName: string, + public readonly requestId: RequestId, + public readonly response: SubmitResponse['response'], + ) { + const cid = Principal.from(canisterId); + super(cid, methodName, 'update', { + 'Request ID': toHex(requestId), + ...(response.body + ? { + ...(response.body.error_code + ? { + 'Error code': response.body.error_code, + } + : {}), + 'Reject code': String(response.body.reject_code), + 'Reject message': response.body.reject_message, + } + : { + 'HTTP status code': response.status.toString(), + 'HTTP status text': response.statusText, + }), + }); + Object.setPrototypeOf(this, UpdateCallRejectedError.prototype); + } +} From 860e4a4152d56a971faedf551918e509baa747a0 Mon Sep 17 00:00:00 2001 From: Kyle Peacock Date: Mon, 22 Jul 2024 16:36:15 -0700 Subject: [PATCH 2/5] fixes observable test --- packages/agent/src/observable.test.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/agent/src/observable.test.ts b/packages/agent/src/observable.test.ts index 116ef9bc6..70a851095 100644 --- a/packages/agent/src/observable.test.ts +++ b/packages/agent/src/observable.test.ts @@ -1,3 +1,4 @@ +import { AgentError } from './errors'; import { Observable, ObservableLog } from './observable'; describe('Observable', () => { @@ -48,7 +49,7 @@ describe('ObservableLog', () => { observable.warn('warning'); expect(observer1).toHaveBeenCalledWith({ message: 'warning', level: 'warn' }); expect(observer2).toHaveBeenCalledWith({ message: 'warning', level: 'warn' }); - const error = new Error('error'); + const error = new AgentError('error'); observable.error('error', error); expect(observer1).toHaveBeenCalledWith({ message: 'error', level: 'error', error }); expect(observer2).toHaveBeenCalledWith({ message: 'error', level: 'error', error }); From e096793caf68fea8933bc6aa4783a8b8e56738da Mon Sep 17 00:00:00 2001 From: Jason I Date: Tue, 22 Oct 2024 18:17:12 -0700 Subject: [PATCH 3/5] chore: cast to v2ResponseBody --- packages/agent/src/errors.ts | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/agent/src/errors.ts b/packages/agent/src/errors.ts index c7aec2b07..deeab1abc 100644 --- a/packages/agent/src/errors.ts +++ b/packages/agent/src/errors.ts @@ -1,12 +1,16 @@ import { Principal } from '@dfinity/principal'; -import { QueryResponseRejected, ReplicaRejectCode, SubmitResponse } from './agent/api'; +import { + QueryResponseRejected, + ReplicaRejectCode, + SubmitResponse, + v2ResponseBody, +} from './agent/api'; import { RequestId } from './request_id'; import { toHex } from './utils/buffer'; /** * An error that happens in the Agent. This is the root of all errors and should be used * everywhere in the Agent code (this package). - * * @todo https://github.com/dfinity/agent-js/issues/420 */ export class AgentError extends Error { @@ -72,13 +76,13 @@ export class UpdateCallRejectedError extends ActorCallError { 'Request ID': toHex(requestId), ...(response.body ? { - ...(response.body.error_code + ...((response.body as v2ResponseBody).error_code ? { - 'Error code': response.body.error_code, + 'Error code': (response.body as v2ResponseBody).error_code, } : {}), - 'Reject code': String(response.body.reject_code), - 'Reject message': response.body.reject_message, + 'Reject code': String((response.body as v2ResponseBody).reject_code), + 'Reject message': (response.body as v2ResponseBody).reject_message, } : { 'HTTP status code': response.status.toString(), From b1a090627c1d98285370c0b30cd9ea7562a77120 Mon Sep 17 00:00:00 2001 From: Jason I Date: Wed, 23 Oct 2024 07:07:17 -0700 Subject: [PATCH 4/5] chore: build fix --- packages/agent/src/agent/http/index.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/agent/src/agent/http/index.ts b/packages/agent/src/agent/http/index.ts index 22346eb6e..c8f5b6572 100644 --- a/packages/agent/src/agent/http/index.ts +++ b/packages/agent/src/agent/http/index.ts @@ -572,7 +572,7 @@ export class HttpAgent implements Agent { ); } - this.log.error('Error while making call:', error as Error); + this.log.error('Error while making call:', error as AgentError); throw error; } } From c418f7e464250ea1a9ecbb27a42fc3d3bf2c62a7 Mon Sep 17 00:00:00 2001 From: Jason I Date: Wed, 23 Oct 2024 08:46:16 -0700 Subject: [PATCH 5/5] chore: prettier --- packages/agent/src/actor.ts | 98 ++++++++++++++++++------------------- 1 file changed, 49 insertions(+), 49 deletions(-) diff --git a/packages/agent/src/actor.ts b/packages/agent/src/actor.ts index 70124d1cc..27833dd3d 100644 --- a/packages/agent/src/actor.ts +++ b/packages/agent/src/actor.ts @@ -24,7 +24,7 @@ export class ActorCallError extends AgentError { constructor( public readonly canisterId: Principal, public readonly methodName: string, - public readonly type: "query" | "update", + public readonly type: 'query' | 'update', public readonly props: Record, ) { super( @@ -33,7 +33,7 @@ export class ActorCallError extends AgentError { ` Canister: ${canisterId.toText()}`, ` Method: ${methodName} (${type})`, ...Object.getOwnPropertyNames(props).map(n => ` "${n}": ${JSON.stringify(props[n])}`), - ].join("\n"), + ].join('\n'), ); } } @@ -44,7 +44,7 @@ export class QueryCallRejectedError extends ActorCallError { methodName: string, public readonly result: QueryResponseRejected, ) { - super(canisterId, methodName, "query", { + super(canisterId, methodName, 'query', { Status: result.status, Code: ReplicaRejectCode[result.reject_code] ?? `Unknown Code "${result.reject_code}"`, Message: result.reject_message, @@ -57,27 +57,27 @@ export class UpdateCallRejectedError extends ActorCallError { canisterId: Principal, methodName: string, public readonly requestId: RequestId, - public readonly response: SubmitResponse["response"], + public readonly response: SubmitResponse['response'], public readonly reject_code: ReplicaRejectCode, public readonly reject_message: string, public readonly error_code?: string, ) { - super(canisterId, methodName, "update", { - "Request ID": toHex(requestId), + super(canisterId, methodName, 'update', { + 'Request ID': toHex(requestId), ...(response.body ? { - ...(error_code - ? { - "Error code": error_code, - } - : {}), - "Reject code": String(reject_code), - "Reject message": reject_message, - } + ...(error_code + ? { + 'Error code': error_code, + } + : {}), + 'Reject code': String(reject_code), + 'Reject message': reject_message, + } : { - "HTTP status code": response.status.toString(), - "HTTP status text": response.statusText, - }), + 'HTTP status code': response.status.toString(), + 'HTTP status text': response.statusText, + }), }); } } @@ -139,7 +139,7 @@ export interface ActorConfig extends CallConfig { /** * Polyfill for BLS Certificate verification in case wasm is not supported */ - blsVerify?: CreateCertificateOptions["blsVerify"]; + blsVerify?: CreateCertificateOptions['blsVerify']; } // TODO: move this to proper typing when Candid support TypeScript. @@ -200,20 +200,20 @@ export type ActorMethodMappedExtended = { */ export type CanisterInstallMode = | { - reinstall: null; -} + reinstall: null; + } | { - upgrade: - | [] - | [ - { - skip_pre_upgrade: [] | [boolean]; - }, - ]; -} + upgrade: + | [] + | [ + { + skip_pre_upgrade: [] | [boolean]; + }, + ]; + } | { - install: null; -}; + install: null; + }; /** * Internal metadata for actors. It's an enhanced version of ActorConfig with @@ -226,7 +226,7 @@ interface ActorMetadata { config: ActorConfig; } -const metadataSymbol = Symbol.for("ic-agent-metadata"); +const metadataSymbol = Symbol.for('ic-agent-metadata'); export interface CreateActorClassOpts { httpDetails?: boolean; @@ -280,7 +280,7 @@ export class Actor { // Same for module. const wasmModule = [...new Uint8Array(fields.module)]; const canisterId = - typeof config.canisterId === "string" + typeof config.canisterId === 'string' ? Principal.fromText(config.canisterId) : config.canisterId; @@ -357,7 +357,7 @@ export class Actor { `Canister ID is required, but received ${typeof config.canisterId} instead. If you are using automatically generated declarations, this may be because your application is not setting the canister ID in process.env correctly.`, ); const canisterId = - typeof config.canisterId === "string" + typeof config.canisterId === 'string' ? Principal.fromText(config.canisterId) : config.canisterId; @@ -462,17 +462,17 @@ const DEFAULT_ACTOR_CONFIG = { export type ActorConstructor = new (config: ActorConfig) => ActorSubclass; -export const ACTOR_METHOD_WITH_HTTP_DETAILS = "http-details"; -export const ACTOR_METHOD_WITH_CERTIFICATE = "certificate"; +export const ACTOR_METHOD_WITH_HTTP_DETAILS = 'http-details'; +export const ACTOR_METHOD_WITH_CERTIFICATE = 'certificate'; function _createActorMethod( actor: Actor, methodName: string, func: IDL.FuncClass, - blsVerify?: CreateCertificateOptions["blsVerify"], + blsVerify?: CreateCertificateOptions['blsVerify'], ): ActorMethod { let caller: (options: CallConfig, ...args: unknown[]) => Promise; - if (func.annotations.includes("query") || func.annotations.includes("composite_query")) { + if (func.annotations.includes('query') || func.annotations.includes('composite_query')) { caller = async (options, ...args) => { // First, if there's a config transformation, call it. options = { @@ -504,9 +504,9 @@ function _createActorMethod( case QueryResponseStatus.Replied: return func.annotations.includes(ACTOR_METHOD_WITH_HTTP_DETAILS) ? { - httpDetails, - result: decodeReturnValue(func.retTypes, result.reply.arg), - } + httpDetails, + result: decodeReturnValue(func.retTypes, result.reply.arg), + } : decodeReturnValue(func.retTypes, result.reply.arg); } }; @@ -631,12 +631,12 @@ function _createActorMethod( } else if (func.retTypes.length === 0) { return shouldIncludeHttpDetails ? { - httpDetails: response, - result: undefined, - } + httpDetails: response, + result: undefined, + } : undefined; } else { - throw new Error(`Call was returned undefined, but type [${func.retTypes.join(",")}].`); + throw new Error(`Call was returned undefined, but type [${func.retTypes.join(',')}].`); } }; } @@ -644,8 +644,8 @@ function _createActorMethod( const handler = (...args: unknown[]) => caller({}, ...args); handler.withOptions = (options: CallConfig) => - (...args: unknown[]) => - caller(options, ...args); + (...args: unknown[]) => + caller(options, ...args); return handler as ActorMethod; } @@ -664,8 +664,8 @@ export function getManagementCanister(config: CallConfig): ActorSubclass(managementCanisterIdl, { ...config, - canisterId: Principal.fromHex(""), + canisterId: Principal.fromHex(''), ...{ callTransform: transform, queryTransform: transform,