From 1779e69c964a6713f8e0ceda61177b6358704a1c Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 11:10:04 +0200 Subject: [PATCH 01/12] chore(open-payments): parse error objects, and return error codes --- packages/open-payments/README.md | 1 + packages/open-payments/src/client/error.ts | 3 + .../open-payments/src/client/requests.test.ts | 150 +++++++++++++++++- packages/open-payments/src/client/requests.ts | 36 +++-- 4 files changed, 180 insertions(+), 10 deletions(-) diff --git a/packages/open-payments/README.md b/packages/open-payments/README.md index ff1fa2ef..73e7b54b 100644 --- a/packages/open-payments/README.md +++ b/packages/open-payments/README.md @@ -104,6 +104,7 @@ try { } catch (error) { if (error instanceof OpenPaymentsClientError) { console.log(error.message) + console.log(error.code) // the error code from the Open Payments API console.log(error.description) // additional description of the error console.log(error.status) // the HTTP status of the request, if a request failure console.log(error.validationErrors) // an array of validation errors. Populated if the response of the request failed OpenAPI specfication validation, or other validation checks. diff --git a/packages/open-payments/src/client/error.ts b/packages/open-payments/src/client/error.ts index 30e642d4..a9e63dc2 100644 --- a/packages/open-payments/src/client/error.ts +++ b/packages/open-payments/src/client/error.ts @@ -1,6 +1,7 @@ interface ErrorDetails { description: string status?: number + code?: string validationErrors?: string[] } @@ -8,12 +9,14 @@ export class OpenPaymentsClientError extends Error { public description: string public validationErrors?: string[] public status?: number + public code?: string constructor(message: string, args: ErrorDetails) { super(message) this.name = 'OpenPaymentsClientError' this.description = args.description this.status = args.status + this.code = args.code this.validationErrors = args.validationErrors } } diff --git a/packages/open-payments/src/client/requests.test.ts b/packages/open-payments/src/client/requests.test.ts index 26366565..375b432b 100644 --- a/packages/open-payments/src/client/requests.test.ts +++ b/packages/open-payments/src/client/requests.test.ts @@ -2,6 +2,7 @@ import { createHttpClient, deleteRequest, get, + handleError, HttpClient, post, requestShouldBeAuthorized, @@ -12,7 +13,7 @@ import nock from 'nock' import { createTestDeps, mockOpenApiResponseValidators } from '../test/helpers' import { OpenPaymentsClientError } from './error' import assert from 'assert' -import ky from 'ky' +import ky, { HTTPError } from 'ky' import { BaseDeps } from '.' const HTTP_SIGNATURE_REGEX = /sig1=:([a-zA-Z0-9+/]){86}==:/ @@ -817,4 +818,151 @@ describe('requests', (): void => { ) }) }) + + describe('handleError', (): void => { + test('handles HTTP error with expected JSON response', async (): Promise => { + const url = 'https://localhost:1000/' + const request = new Request(url) + const response = new Response( + JSON.stringify({ + error: { + code: 'invalid_client', + description: 'Could not determine client' + } + }), + { status: 400 } + ) + + expect.assertions(5) + try { + await handleError(deps, { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + error: new HTTPError(response, request, undefined!), + requestType: 'POST', + url + }) + } catch (error) { + assert.ok(error instanceof OpenPaymentsClientError) + expect(error.message).toBe('Error making Open Payments POST request') + expect(error.description).toBe('Could not determine client') + expect(error.code).toBe('invalid_client') + expect(error.status).toBe(400) + expect(error.validationErrors).toBeUndefined() + } + }) + + test('handles HTTP error with unexpected JSON response', async (): Promise => { + const url = 'https://localhost:1000/' + const request = new Request(url) + const responseBody = { + unexpected: 'response' + } + const response = new Response(JSON.stringify(responseBody), { + status: 400 + }) + + expect.assertions(5) + try { + await handleError(deps, { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + error: new HTTPError(response, request, undefined!), + requestType: 'POST', + url + }) + } catch (error) { + assert.ok(error instanceof OpenPaymentsClientError) + expect(error.message).toBe('Error making Open Payments POST request') + expect(error.description).toBe(JSON.stringify(responseBody)) + expect(error.code).toBeUndefined() + expect(error.status).toBe(400) + expect(error.validationErrors).toBeUndefined() + } + }) + + test('handles HTTP error with text response', async (): Promise => { + const url = 'https://localhost:1000/' + const request = new Request(url) + const response = new Response('Bad Request', { status: 400 }) + + expect.assertions(5) + try { + await handleError(deps, { + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + error: new HTTPError(response, request, undefined!), + requestType: 'POST', + url + }) + } catch (error) { + assert.ok(error instanceof OpenPaymentsClientError) + expect(error.message).toBe('Error making Open Payments POST request') + expect(error.description).toBe('Bad Request') + expect(error.code).toBeUndefined() + expect(error.status).toBe(400) + expect(error.validationErrors).toBeUndefined() + } + }) + + test('handles validation error', async (): Promise => { + const url = 'https://localhost:1000/' + + expect.assertions(5) + try { + await handleError(deps, { + error: { + status: 400, + errors: [{ message: 'invalid request' }] + }, + requestType: 'POST', + url + }) + } catch (error) { + assert.ok(error instanceof OpenPaymentsClientError) + expect(error.message).toBe('Error making Open Payments POST request') + expect(error.description).toBe('Could not validate OpenAPI response') + expect(error.code).toBeUndefined() + expect(error.status).toBe(400) + expect(error.validationErrors).toEqual(['invalid request']) + } + }) + + test('handles ordinary error', async (): Promise => { + const url = 'https://localhost:1000/' + + expect.assertions(5) + try { + await handleError(deps, { + error: new Error('Something went wrong'), + requestType: 'POST', + url + }) + } catch (error) { + assert.ok(error instanceof OpenPaymentsClientError) + expect(error.message).toBe('Error making Open Payments POST request') + expect(error.description).toBe('Something went wrong') + expect(error.code).toBeUndefined() + expect(error.status).toBeUndefined() + expect(error.validationErrors).toBeUndefined() + } + }) + + test('handles unexpected (non-object) error', async (): Promise => { + const url = 'https://localhost:1000/' + + expect.assertions(5) + try { + await handleError(deps, { + error: 'unexpected error', + requestType: 'POST', + url + }) + } catch (error) { + assert.ok(error instanceof OpenPaymentsClientError) + expect(error.message).toBe('Error making Open Payments POST request') + expect(error.description).toBe('Received unexpected error') + expect(error.code).toBeUndefined() + expect(error.status).toBeUndefined() + expect(error.validationErrors).toBeUndefined() + } + }) + }) }) diff --git a/packages/open-payments/src/client/requests.ts b/packages/open-payments/src/client/requests.ts index 79437408..ef426dda 100644 --- a/packages/open-payments/src/client/requests.ts +++ b/packages/open-payments/src/client/requests.ts @@ -149,7 +149,7 @@ interface HandleErrorArgs { requestType: 'POST' | 'DELETE' | 'GET' } -const handleError = async ( +export const handleError = async ( deps: BaseDeps, args: HandleErrorArgs ): Promise => { @@ -158,23 +158,34 @@ const handleError = async ( let errorDescription let errorStatus let validationErrors + let errorCode const { HTTPError } = await import('ky') if (error instanceof HTTPError) { - let responseBody + let responseBody: + | { + error: { description: string; code: string } + } + | string + | undefined try { - responseBody = (await error.response.json()) as { message: string } + responseBody = await error.response.text() + responseBody = JSON.parse(responseBody) } catch { // Ignore if we can't parse the response body (or no body exists) } + errorStatus = error.response.status errorDescription = - responseBody && responseBody.message - ? responseBody.message - : error.message - errorStatus = error.response?.status + typeof responseBody === 'object' + ? responseBody.error?.description || JSON.stringify(responseBody) + : responseBody || error.message + errorCode = + typeof responseBody === 'object' && responseBody.error?.code + ? responseBody.error.code + : undefined } else if (isValidationError(error)) { errorDescription = 'Could not validate OpenAPI response' validationErrors = error.errors.map((e) => e.message) @@ -188,14 +199,21 @@ const handleError = async ( const errorMessage = `Error making Open Payments ${requestType} request` deps.logger.error( - { status: errorStatus, errorDescription, url, requestType }, + { + method: requestType, + url, + status: errorStatus, + description: errorDescription, + code: errorCode + }, errorMessage ) throw new OpenPaymentsClientError(errorMessage, { description: errorDescription, validationErrors, - status: errorStatus + status: errorStatus, + code: errorCode }) } From 1225de3604a17c72bbf1daf09b3f5960edc9ca24 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:13:20 +0200 Subject: [PATCH 02/12] chore(open-payments): add missing assertions, update tests --- packages/open-payments/src/client/index.test.ts | 3 +++ packages/open-payments/src/client/outgoing-payment.test.ts | 6 ++++-- 2 files changed, 7 insertions(+), 2 deletions(-) diff --git a/packages/open-payments/src/client/index.test.ts b/packages/open-payments/src/client/index.test.ts index 4e850bb6..02978b19 100644 --- a/packages/open-payments/src/client/index.test.ts +++ b/packages/open-payments/src/client/index.test.ts @@ -75,6 +75,7 @@ describe('Client', (): void => { }) test('throws error if could not load private key as Buffer', async (): Promise => { + expect.assertions(2) try { await createAuthenticatedClient({ logger: silentLogger, @@ -92,6 +93,7 @@ describe('Client', (): void => { }) test('throws error if could not load private key', async (): Promise => { + expect.assertions(2) try { await createAuthenticatedClient({ logger: silentLogger, @@ -116,6 +118,7 @@ describe('Client', (): void => { `( 'throws an error if both authenticatedRequestInterceptor and privateKey or keyId are provided', async ({ keyId, privateKey }) => { + expect.assertions(2) try { // @ts-expect-error Invalid args await createAuthenticatedClient({ diff --git a/packages/open-payments/src/client/outgoing-payment.test.ts b/packages/open-payments/src/client/outgoing-payment.test.ts index e25d695a..f22b2025 100644 --- a/packages/open-payments/src/client/outgoing-payment.test.ts +++ b/packages/open-payments/src/client/outgoing-payment.test.ts @@ -235,6 +235,7 @@ describe('outgoing-payment', (): void => { .query({ 'wallet-address': walletAddress }) .reply(200, outgoingPaymentPaginationResult) + expect.assertions(3) try { await listOutgoingPayments( deps, @@ -278,7 +279,7 @@ describe('outgoing-payment', (): void => { }, openApiValidators.failedValidator ) - ).rejects.toThrowError() + ).rejects.toThrow() scope.done() }) }) @@ -355,6 +356,7 @@ describe('outgoing-payment', (): void => { .post('/outgoing-payments') .reply(200, outgoingPayment) + expect.assertions(3) try { await createOutgoingPayment( deps, @@ -402,7 +404,7 @@ describe('outgoing-payment', (): void => { walletAddress } ) - ).rejects.toThrowError(OpenPaymentsClientError) + ).rejects.toThrow(OpenPaymentsClientError) scope.done() }) }) From f04588b759d965fa9001b38bb35ae3750bc9e7d5 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:22:05 +0200 Subject: [PATCH 03/12] chore(open-payments): update README --- packages/open-payments/README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/open-payments/README.md b/packages/open-payments/README.md index 73e7b54b..c2d02cd9 100644 --- a/packages/open-payments/README.md +++ b/packages/open-payments/README.md @@ -104,9 +104,9 @@ try { } catch (error) { if (error instanceof OpenPaymentsClientError) { console.log(error.message) - console.log(error.code) // the error code from the Open Payments API console.log(error.description) // additional description of the error console.log(error.status) // the HTTP status of the request, if a request failure + console.log(error.code) // the error code from the Open Payments API console.log(error.validationErrors) // an array of validation errors. Populated if the response of the request failed OpenAPI specfication validation, or other validation checks. } else { console.log(error) From 7cbcfe2cc08d017d8a7e8a8fae2559848514c1ef Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:22:12 +0200 Subject: [PATCH 04/12] chore(open-payments): add changeset --- .changeset/warm-geese-rescue.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/warm-geese-rescue.md diff --git a/.changeset/warm-geese-rescue.md b/.changeset/warm-geese-rescue.md new file mode 100644 index 00000000..6a755e08 --- /dev/null +++ b/.changeset/warm-geese-rescue.md @@ -0,0 +1,5 @@ +--- +'@interledger/open-payments': minor +--- + +Adding functionality to parse error objects from Open Payments API responses, and expose new `code` field in `OpenPaymentsClientError`. From 8f8f9e0646377f159cff533abd6979f88189d9ce Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:50:31 +0200 Subject: [PATCH 05/12] feat(open-payments): adding logging request and response hooks for http client --- .../open-payments/src/client/requests.test.ts | 76 ++++++++++--- packages/open-payments/src/client/requests.ts | 107 +++++++++++++----- packages/open-payments/src/test/helpers.ts | 7 +- 3 files changed, 143 insertions(+), 47 deletions(-) diff --git a/packages/open-payments/src/client/requests.test.ts b/packages/open-payments/src/client/requests.test.ts index 375b432b..5bc9725f 100644 --- a/packages/open-payments/src/client/requests.test.ts +++ b/packages/open-payments/src/client/requests.test.ts @@ -10,7 +10,11 @@ import { } from './requests' import { generateKeyPairSync } from 'crypto' import nock from 'nock' -import { createTestDeps, mockOpenApiResponseValidators } from '../test/helpers' +import { + createTestDeps, + mockOpenApiResponseValidators, + silentLogger +} from '../test/helpers' import { OpenPaymentsClientError } from './error' import assert from 'assert' import ky, { HTTPError } from 'ky' @@ -39,9 +43,12 @@ describe('requests', (): void => { beforeAll(async () => { httpClient = await createHttpClient({ + logger: silentLogger, requestTimeoutMs: 1000000, - privateKey, - keyId + requestSigningArgs: { + privateKey, + keyId + } }) deps = await createTestDeps({ httpClient @@ -52,7 +59,14 @@ describe('requests', (): void => { test('sets timeout properly', async (): Promise => { const kyCreateSpy = jest.spyOn(ky, 'create') - await createHttpClient({ requestTimeoutMs: 1000, privateKey, keyId }) + await createHttpClient({ + logger: silentLogger, + requestTimeoutMs: 1000, + requestSigningArgs: { + privateKey, + keyId + } + }) expect(kyCreateSpy).toHaveBeenCalledWith( expect.objectContaining({ timeout: 1000 }) @@ -61,7 +75,14 @@ describe('requests', (): void => { test('sets headers properly', async (): Promise => { const kyCreateSpy = jest.spyOn(ky, 'create') - await createHttpClient({ requestTimeoutMs: 1000, privateKey, keyId }) + await createHttpClient({ + logger: silentLogger, + requestTimeoutMs: 1000, + requestSigningArgs: { + privateKey, + keyId + } + }) expect(kyCreateSpy).toHaveBeenCalledWith( expect.objectContaining({ @@ -81,15 +102,24 @@ describe('requests', (): void => { const kyExtendSpy = jest.spyOn(kyInstance, 'extend') await createHttpClient({ + logger: silentLogger, requestTimeoutMs: 0, - authenticatedRequestInterceptor: (config) => config - }) - - expect(kyExtendSpy).toHaveBeenCalledWith({ - hooks: { - beforeRequest: [expect.any(Function)] + requestSigningArgs: { + privateKey, + keyId } }) + + assert.ok(kyExtendSpy.mock.calls[0][0].hooks?.beforeRequest) + expect(kyExtendSpy.mock.calls[0][0].hooks?.beforeRequest).toHaveLength(2) + + const requestSignerWithPrivateKey = + kyExtendSpy.mock.calls[0][0].hooks.beforeRequest[1] + + const request = new Request('http://localhost:1000') + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + expect(requestSignerWithPrivateKey(request, undefined!)).toBeDefined() }) test('sets authenticated request interceptor', async (): Promise => { @@ -99,16 +129,28 @@ describe('requests', (): void => { const kyExtendSpy = jest.spyOn(kyInstance, 'extend') + const mockedAuthenticatedRequestInterceptor = jest.fn() + await createHttpClient({ + logger: silentLogger, requestTimeoutMs: 0, - authenticatedRequestInterceptor: (config) => config - }) - - expect(kyExtendSpy).toHaveBeenCalledWith({ - hooks: { - beforeRequest: [expect.any(Function)] + requestSigningArgs: { + authenticatedRequestInterceptor: mockedAuthenticatedRequestInterceptor } }) + + assert.ok(kyExtendSpy.mock.calls[0][0].hooks?.beforeRequest) + expect(kyExtendSpy.mock.calls[0][0].hooks?.beforeRequest).toHaveLength(2) + + const authenticatedRequestInterceptor = + kyExtendSpy.mock.calls[0][0].hooks.beforeRequest[1] + + const request = new Request('http://localhost:1000', { method: 'POST' }) + + // eslint-disable-next-line @typescript-eslint/no-non-null-assertion + authenticatedRequestInterceptor(request, undefined!) + + expect(mockedAuthenticatedRequestInterceptor).toHaveBeenCalled() }) }) diff --git a/packages/open-payments/src/client/requests.ts b/packages/open-payments/src/client/requests.ts index ef426dda..02ce234a 100644 --- a/packages/open-payments/src/client/requests.ts +++ b/packages/open-payments/src/client/requests.ts @@ -3,9 +3,10 @@ import { ResponseValidator, isValidationError } from '@interledger/openapi' import { BaseDeps } from '.' import { createHeaders } from '@interledger/http-signature-utils' import { OpenPaymentsClientError } from './error' +import { Logger } from 'pino' // @ts-expect-error We know we are importing an ESM module into our CJS file, so ignore warnings for types -import type { KyInstance } from 'ky' +import type { KyInstance, NormalizedOptions } from 'ky' interface GetArgs { url: string @@ -226,61 +227,111 @@ const checkUrlProtocol = (deps: BaseDeps, url: string): string => { return requestUrl.href } -type CreateHttpClientArgs = { +interface CreateHttpClientArgs { + logger: Logger requestTimeoutMs: number -} & ( - | { privateKey?: KeyObject; keyId?: string } + requestSigningArgs?: AuthenticatedHttpClientArgs +} + +type AuthenticatedHttpClientArgs = + | { privateKey: KeyObject; keyId: string } | { authenticatedRequestInterceptor: InterceptorFn } -) export type HttpClient = KyInstance -export type InterceptorFn = (request: Request) => Request | Promise +export type InterceptorFn = ( + request: Request +) => Request | Promise | void | Promise export const createHttpClient = async ( args: CreateHttpClientArgs ): Promise => { const { default: ky } = await import('ky') + const { requestTimeoutMs, requestSigningArgs, logger } = args + const kyInstance = ky.create({ - timeout: args.requestTimeoutMs, + timeout: requestTimeoutMs, headers: { Accept: 'application/json', 'Content-Type': 'application/json' } }) - let requestInterceptor: InterceptorFn | undefined + const beforeRequestHooks = [] + const afterResponseHooks = [] - if ('authenticatedRequestInterceptor' in args) { - requestInterceptor = (request) => { - if (requestShouldBeAuthorized(request)) { - return args.authenticatedRequestInterceptor(request) - } + const requestLogger: InterceptorFn = async (request) => { + const requestBody = request.body ? await request.clone().json() : undefined + + logger.debug( + { + method: request.method, + url: request.url, + body: requestBody + }, + 'Sending request' + ) + } - return request + const responseLogger = async ( + request: Request, + _: NormalizedOptions, + response: Response + ) => { + let responseBody + try { + responseBody = await response.clone().text() + responseBody = JSON.parse(responseBody) + } catch { + // Ignore if we can't parse the response body (or no body exists) } - } else { - requestInterceptor = (request) => { - const { privateKey, keyId } = args - if (requestShouldBeAuthorized(request)) { - return signRequest(request, { privateKey, keyId }) + logger.debug( + { + method: request.method, + url: response.url, + body: responseBody, + status: response.status + }, + 'Received response' + ) + } + + beforeRequestHooks.push(requestLogger) + afterResponseHooks.push(responseLogger) + + if (requestSigningArgs) { + let requestInterceptor: InterceptorFn | undefined + if ('authenticatedRequestInterceptor' in requestSigningArgs) { + requestInterceptor = (request) => { + if (requestShouldBeAuthorized(request)) { + return requestSigningArgs.authenticatedRequestInterceptor(request) + } + + return request } + } else { + requestInterceptor = (request) => { + const { privateKey, keyId } = requestSigningArgs - return request - } - } + if (requestShouldBeAuthorized(request)) { + return signRequest(request, { privateKey, keyId }) + } - if (requestInterceptor) { - return kyInstance.extend({ - hooks: { - beforeRequest: [requestInterceptor] + return request } - }) + } + + beforeRequestHooks.push(requestInterceptor) } - return kyInstance + return kyInstance.extend({ + hooks: { + beforeRequest: beforeRequestHooks, + afterResponse: afterResponseHooks + } + }) } export const requestShouldBeAuthorized = (request: Request) => diff --git a/packages/open-payments/src/test/helpers.ts b/packages/open-payments/src/test/helpers.ts index fae8e328..c27cdf5c 100644 --- a/packages/open-payments/src/test/helpers.ts +++ b/packages/open-payments/src/test/helpers.ts @@ -35,9 +35,12 @@ export const getDefaultHttpClient = async (): ReturnType< typeof createHttpClient > => createHttpClient({ + logger: silentLogger, requestTimeoutMs: 1000, - keyId, - privateKey: generateKeyPairSync('ed25519').privateKey + requestSigningArgs: { + keyId, + privateKey: generateKeyPairSync('ed25519').privateKey + } }) export const mockOpenApiResponseValidators = () => ({ From 00d17f060a35f1dbdb4ae82f82120c231d2ac338 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:51:42 +0200 Subject: [PATCH 06/12] feat(open-payments): adding logging during client initialization --- .../open-payments/src/client/index.test.ts | 4 +- packages/open-payments/src/client/index.ts | 94 ++++++++++++------- 2 files changed, 64 insertions(+), 34 deletions(-) diff --git a/packages/open-payments/src/client/index.test.ts b/packages/open-payments/src/client/index.test.ts index 02978b19..b7b5c950 100644 --- a/packages/open-payments/src/client/index.test.ts +++ b/packages/open-payments/src/client/index.test.ts @@ -86,7 +86,7 @@ describe('Client', (): void => { } catch (error) { assert.ok(error instanceof OpenPaymentsClientError) expect(error.message).toBe( - 'Could not load private key when creating Open Payments client' + 'Could not load private key when creating authenticated client' ) expect(error.description).toBe('Key is not a valid file') } @@ -104,7 +104,7 @@ describe('Client', (): void => { } catch (error) { assert.ok(error instanceof OpenPaymentsClientError) expect(error.message).toBe( - 'Could not load private key when creating Open Payments client' + 'Could not load private key when creating authenticated client' ) expect(error.description).toBe('Key is not a valid path or file') } diff --git a/packages/open-payments/src/client/index.ts b/packages/open-payments/src/client/index.ts index 88ea2ff7..61367ccf 100644 --- a/packages/open-payments/src/client/index.ts +++ b/packages/open-payments/src/client/index.ts @@ -97,31 +97,31 @@ export interface CollectionRequestArgs walletAddress: string } -const parseKey = ( - args: Partial -): KeyObject | undefined => { - if (!args.privateKey) { - return undefined +const parseKey = (deps: { logger: Logger }, privateKey: KeyLike): KeyObject => { + if (privateKey instanceof KeyObject) { + deps.logger.debug('Loading key from KeyObject') + return privateKey } - if (args.privateKey instanceof KeyObject) { - return args.privateKey - } - - if (args.privateKey instanceof Buffer) { + if (privateKey instanceof Buffer) { try { - return createPrivateKey(args.privateKey) + deps.logger.debug('Loading key from Buffer') + return createPrivateKey(privateKey) } catch { throw new Error('Key is not a valid file') } } - if (fs.existsSync(path.resolve(process.cwd(), args.privateKey))) { - return loadKey(path.resolve(process.cwd(), args.privateKey)) + const keyFilePath = path.resolve(process.cwd(), privateKey) + + if (fs.existsSync(keyFilePath)) { + deps.logger.debug(`Loading key from file path: ${keyFilePath}`) + return loadKey(keyFilePath) } try { - return createPrivateKey(args.privateKey) + deps.logger.debug('Loading key from string') + return createPrivateKey(privateKey) } catch { throw new Error('Key is not a valid path or file') } @@ -138,6 +138,7 @@ const createUnauthenticatedDeps = async ({ } const httpClient = await createHttpClient({ + logger, requestTimeoutMs: args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS }) @@ -171,35 +172,42 @@ const createAuthenticatedClientDeps = async ({ logger.level = args.logLevel } - let privateKey: KeyObject | undefined - try { - privateKey = parseKey(args) - } catch (error) { - const errorMessage = - 'Could not load private key when creating Open Payments client' - const description = error instanceof Error ? error.message : 'Unknown error' - - logger.error({ description }, errorMessage) - - throw new OpenPaymentsClientError(errorMessage, { - description - }) - } - let httpClient: HttpClient if ('authenticatedRequestInterceptor' in args) { httpClient = await createHttpClient({ + logger, requestTimeoutMs: args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS, - authenticatedRequestInterceptor: args.authenticatedRequestInterceptor + requestSigningArgs: { + authenticatedRequestInterceptor: args.authenticatedRequestInterceptor + } }) } else { + let privateKey: KeyObject + try { + privateKey = parseKey({ logger }, args.privateKey) + } catch (error) { + const errorMessage = + 'Could not load private key when creating authenticated client' + const description = + error instanceof Error ? error.message : 'Unknown error' + + logger.error({ description }, errorMessage) + + throw new OpenPaymentsClientError(errorMessage, { + description + }) + } + httpClient = await createHttpClient({ + logger, requestTimeoutMs: args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS, - privateKey, - keyId: args.keyId + requestSigningArgs: { + privateKey, + keyId: args.keyId + } }) } @@ -250,6 +258,16 @@ export const createUnauthenticatedClient = async ( const { resourceServerOpenApi, walletAddressServerOpenApi, ...baseDeps } = await createUnauthenticatedDeps(args) + baseDeps.logger.debug( + { + validateResponses: !!args.validateResponses, + useHttp: baseDeps.useHttp, + requestTimeoutMs: + args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS + }, + 'Created unauthenticated client' + ) + return { walletAddress: createWalletAddressRoutes({ ...baseDeps, @@ -335,6 +353,18 @@ export async function createAuthenticatedClient( ...baseDeps } = await createAuthenticatedClientDeps(args) + baseDeps.logger.debug( + { + walletAddressUrl: args.walletAddressUrl, + ...('keyId' in args ? { keyId: args.keyId } : {}), + validateResponses: !!args.validateResponses, + useHttp: baseDeps.useHttp, + requestTimeoutMs: + args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS + }, + 'Created authenticated client' + ) + return { incomingPayment: createIncomingPaymentRoutes({ ...baseDeps, From ada7c15b762700f76da60182a7c2912edef0e8d9 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:51:58 +0200 Subject: [PATCH 07/12] chore(open-payments): make default logging silent --- packages/open-payments/README.md | 2 +- packages/open-payments/src/client/index.ts | 8 ++++++-- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/packages/open-payments/README.md b/packages/open-payments/README.md index c2d02cd9..cc9aec97 100644 --- a/packages/open-payments/README.md +++ b/packages/open-payments/README.md @@ -56,7 +56,7 @@ const incomingPayment = await client.walletAddress.get({ | ------------------- | --------------------------------------------------------------------------------------------------------------- | | `requestTimeoutMs` | (optional) The timeout in ms for each request by the client. Defaults to 5000. | | `logger` | (optional) The custom logger to provide for the client. Defaults to pino logger. | -| `logLevel` | (optional) The log level for the client. Defaults to `info`. | +| `logLevel` | (optional) The log level for the client. Defaults to `silent`. | | `validateResponses` | (optional) Enables or disables response validation against the Open Payments OpenAPI specs. Defaults to `true`. | ### `AuthenticatedClient` diff --git a/packages/open-payments/src/client/index.ts b/packages/open-payments/src/client/index.ts index 61367ccf..ea41133d 100644 --- a/packages/open-payments/src/client/index.ts +++ b/packages/open-payments/src/client/index.ts @@ -132,7 +132,9 @@ const createUnauthenticatedDeps = async ({ validateResponses = true, ...args }: Partial = {}): Promise => { - const logger = args?.logger ?? createLogger({ name: 'Open Payments Client' }) + const logger = + args?.logger ?? + createLogger({ name: 'Open Payments Client', level: 'silent' }) if (args.logLevel) { logger.level = args.logLevel } @@ -167,7 +169,9 @@ const createAuthenticatedClientDeps = async ({ }: | CreateAuthenticatedClientArgs | CreateAuthenticatedClientWithReqInterceptorArgs): Promise => { - const logger = args?.logger ?? createLogger({ name: 'Open Payments Client' }) + const logger = + args?.logger ?? + createLogger({ name: 'Open Payments Client', level: 'silent' }) if (args.logLevel) { logger.level = args.logLevel } From 0080304bf993abf44ffad4f759109a52aaba8c62 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 12:53:55 +0200 Subject: [PATCH 08/12] chore(open-payments): add changeset --- .changeset/brave-pots-rest.md | 7 +++++++ 1 file changed, 7 insertions(+) create mode 100644 .changeset/brave-pots-rest.md diff --git a/.changeset/brave-pots-rest.md b/.changeset/brave-pots-rest.md new file mode 100644 index 00000000..821c6120 --- /dev/null +++ b/.changeset/brave-pots-rest.md @@ -0,0 +1,7 @@ +--- +'@interledger/open-payments': minor +--- + +- Adding debug logs during client initialization +- Adding debug logs for requests and responses in the http client +- Making log level `silent` by default From 7a78eb91eb0c232e21aa349c5a84a5b8c99b798b Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 13:22:45 +0200 Subject: [PATCH 09/12] chore(open-payments): simplify assertion --- packages/open-payments/src/client/requests.ts | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/packages/open-payments/src/client/requests.ts b/packages/open-payments/src/client/requests.ts index ef426dda..f8811184 100644 --- a/packages/open-payments/src/client/requests.ts +++ b/packages/open-payments/src/client/requests.ts @@ -183,9 +183,7 @@ export const handleError = async ( ? responseBody.error?.description || JSON.stringify(responseBody) : responseBody || error.message errorCode = - typeof responseBody === 'object' && responseBody.error?.code - ? responseBody.error.code - : undefined + typeof responseBody === 'object' ? responseBody.error?.code : undefined } else if (isValidationError(error)) { errorDescription = 'Could not validate OpenAPI response' validationErrors = error.errors.map((e) => e.message) From d91c08d2ad586405fb9a4ac1398057fd35f4e8ad Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Wed, 3 Jul 2024 14:00:18 +0200 Subject: [PATCH 10/12] chore(open-payments): remove old changeset --- .changeset/warm-geese-rescue.md | 5 ----- 1 file changed, 5 deletions(-) delete mode 100644 .changeset/warm-geese-rescue.md diff --git a/.changeset/warm-geese-rescue.md b/.changeset/warm-geese-rescue.md deleted file mode 100644 index 6a755e08..00000000 --- a/.changeset/warm-geese-rescue.md +++ /dev/null @@ -1,5 +0,0 @@ ---- -'@interledger/open-payments': minor ---- - -Adding functionality to parse error objects from Open Payments API responses, and expose new `code` field in `OpenPaymentsClientError`. From 6515675c0fc908b40a1ebc2a1be63b58cf23573e Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Thu, 4 Jul 2024 14:01:59 +0200 Subject: [PATCH 11/12] chore(open-payments): remove optional chaining --- packages/open-payments/src/client/index.ts | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/packages/open-payments/src/client/index.ts b/packages/open-payments/src/client/index.ts index ea41133d..a045988e 100644 --- a/packages/open-payments/src/client/index.ts +++ b/packages/open-payments/src/client/index.ts @@ -133,7 +133,7 @@ const createUnauthenticatedDeps = async ({ ...args }: Partial = {}): Promise => { const logger = - args?.logger ?? + args.logger ?? createLogger({ name: 'Open Payments Client', level: 'silent' }) if (args.logLevel) { logger.level = args.logLevel @@ -141,8 +141,7 @@ const createUnauthenticatedDeps = async ({ const httpClient = await createHttpClient({ logger, - requestTimeoutMs: - args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS + requestTimeoutMs: args.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS }) let walletAddressServerOpenApi: OpenAPI | undefined @@ -170,7 +169,7 @@ const createAuthenticatedClientDeps = async ({ | CreateAuthenticatedClientArgs | CreateAuthenticatedClientWithReqInterceptorArgs): Promise => { const logger = - args?.logger ?? + args.logger ?? createLogger({ name: 'Open Payments Client', level: 'silent' }) if (args.logLevel) { logger.level = args.logLevel @@ -182,7 +181,7 @@ const createAuthenticatedClientDeps = async ({ httpClient = await createHttpClient({ logger, requestTimeoutMs: - args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS, + args.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS, requestSigningArgs: { authenticatedRequestInterceptor: args.authenticatedRequestInterceptor } @@ -207,7 +206,7 @@ const createAuthenticatedClientDeps = async ({ httpClient = await createHttpClient({ logger, requestTimeoutMs: - args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS, + args.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS, requestSigningArgs: { privateKey, keyId: args.keyId @@ -267,7 +266,7 @@ export const createUnauthenticatedClient = async ( validateResponses: !!args.validateResponses, useHttp: baseDeps.useHttp, requestTimeoutMs: - args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS + args.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS }, 'Created unauthenticated client' ) @@ -364,7 +363,7 @@ export async function createAuthenticatedClient( validateResponses: !!args.validateResponses, useHttp: baseDeps.useHttp, requestTimeoutMs: - args?.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS + args.requestTimeoutMs ?? config.DEFAULT_REQUEST_TIMEOUT_MS }, 'Created authenticated client' ) From 2a9b27f101cff5603206604282c10f5bc67e9c98 Mon Sep 17 00:00:00 2001 From: Max Kurapov Date: Thu, 4 Jul 2024 14:12:48 +0200 Subject: [PATCH 12/12] chore(ci): update pnpm action --- .github/workflows/deploy-docs.yaml | 3 +-- .github/workflows/env-setup/action.yaml | 4 +--- .github/workflows/test-docs-build.yml | 3 +-- 3 files changed, 3 insertions(+), 7 deletions(-) diff --git a/.github/workflows/deploy-docs.yaml b/.github/workflows/deploy-docs.yaml index 7a8b321e..edb64ee9 100644 --- a/.github/workflows/deploy-docs.yaml +++ b/.github/workflows/deploy-docs.yaml @@ -26,10 +26,9 @@ jobs: with: node-version: 20 - - uses: pnpm/action-setup@v2 + - uses: pnpm/action-setup@v4 name: Install pnpm with: - version: 8 run_install: false - name: Install dependencies diff --git a/.github/workflows/env-setup/action.yaml b/.github/workflows/env-setup/action.yaml index 7ad240dc..b91a2a94 100644 --- a/.github/workflows/env-setup/action.yaml +++ b/.github/workflows/env-setup/action.yaml @@ -4,9 +4,7 @@ description: "Sets node version, init pnpm, restore cache" runs: using: "composite" steps: - - uses: pnpm/action-setup@v2 - with: - version: 8 + - uses: pnpm/action-setup@v4 - uses: actions/setup-node@v3 with: node-version: 20 diff --git a/.github/workflows/test-docs-build.yml b/.github/workflows/test-docs-build.yml index ccbcc7c9..e07c1959 100644 --- a/.github/workflows/test-docs-build.yml +++ b/.github/workflows/test-docs-build.yml @@ -20,10 +20,9 @@ jobs: with: node-version: 20 - - uses: pnpm/action-setup@v2 + - uses: pnpm/action-setup@v4 name: Install pnpm with: - version: 8 run_install: false - name: Install dependencies