From cf57ae040502adf02ff274aaba9912bf923a27c1 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 17:29:05 +0000 Subject: [PATCH 1/9] Type WebServiceError.code with an open ClientErrorCode union Replace the plain `string` type on `code` with `WebServiceErrorCode`, an open `ClientErrorCode | (string & {})` union, mirroring geoip2-node. This offers autocompletion for the five client-generated codes while still accepting any code the web service returns. The `ClientErrorCode` and `WebServiceErrorCode` types are exported from the package. Client-generated errors are now built through a `clientError()` helper typed with the closed `ClientErrorCode`, so a typo at a throw site is a compile error. The server-returned `data.code` path stays open. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/errors.ts | 6 +++--- src/index.ts | 7 +++++-- src/types.ts | 35 ++++++++++++++++++++++++++++++++++- src/webServiceClient.ts | 35 ++++++++++++++++++++++++----------- 4 files changed, 66 insertions(+), 17 deletions(-) diff --git a/src/errors.ts b/src/errors.ts index 9ca2473c..f610e2ce 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,4 +1,4 @@ -import { WebServiceClientError } from './types.js'; +import { WebServiceClientError, WebServiceErrorCode } from './types.js'; /* tslint:disable:max-classes-per-file */ export class ArgumentError extends Error { @@ -21,7 +21,7 @@ export class WebServiceError extends Error implements WebServiceClientError { /** * The error code returned by the web service or generated by this client. */ - public readonly code: string; + public readonly code: WebServiceErrorCode; /** * A human-readable description of the error. This is an alias of `message`, * retained for backward compatibility. @@ -42,7 +42,7 @@ export class WebServiceError extends Error implements WebServiceClientError { constructor( properties: { - code: string; + code: WebServiceErrorCode; error: string; status?: number; url: string; diff --git a/src/index.ts b/src/index.ts index 316e9859..e7fab990 100644 --- a/src/index.ts +++ b/src/index.ts @@ -13,7 +13,6 @@ import Shipping from './request/shipping.js'; import ShoppingCartItem from './request/shopping-cart-item.js'; import Transaction from './request/transaction.js'; import TransactionReport from './request/transaction-report.js'; -import { WebServiceClientError } from './types.js'; import Client from './webServiceClient.js'; export { @@ -36,4 +35,8 @@ export { WebServiceError, }; -export type { WebServiceClientError }; +export type { + ClientErrorCode, + WebServiceClientError, + WebServiceErrorCode, +} from './types.js'; diff --git a/src/types.ts b/src/types.ts index 4d21304b..5c50dbd2 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,7 +1,40 @@ +/** + * The error codes that this client generates itself, as opposed to those + * returned by the web service. + */ +export type ClientErrorCode = + | 'FETCH_ERROR' + | 'HTTP_STATUS_CODE_ERROR' + | 'INVALID_RESPONSE_BODY' + | 'NETWORK_TIMEOUT' + | 'SERVER_ERROR'; + +/** + * The `code` exposed on a {@link WebServiceError}. This is one of the + * client-generated {@link ClientErrorCode} values or any other string returned + * by the web service. The `& {}` keeps the union open to arbitrary strings + * while still offering autocompletion for the known client codes. + */ +export type WebServiceErrorCode = ClientErrorCode | (string & {}); + export interface WebServiceClientError { - code: string; + /** + * The error code returned by the web service or generated by this client. + */ + code: WebServiceErrorCode; + /** + * A human-readable description of the error. This is an alias of the standard + * `Error` `message`. + */ error: string; + /** + * The HTTP status code, when the error originated from an HTTP response. + * Absent for network-level errors. + */ status?: number; + /** + * The URL that was being requested when the error occurred. + */ url: string; /** * The underlying error that caused this one, when available (for example, diff --git a/src/webServiceClient.ts b/src/webServiceClient.ts index df507ee3..d1d4c468 100644 --- a/src/webServiceClient.ts +++ b/src/webServiceClient.ts @@ -3,13 +3,29 @@ import { WebServiceError } from './errors.js'; import Transaction from './request/transaction.js'; import TransactionReport from './request/transaction-report.js'; import * as models from './response/models/index.js'; +import { ClientErrorCode } from './types.js'; type servicePath = 'factors' | 'insights' | 'score' | 'transactions/report'; const invalidResponseBody = { code: 'INVALID_RESPONSE_BODY', error: 'Received an invalid or unparseable response body', -}; +} satisfies { code: ClientErrorCode; error: string }; + +// Builds a WebServiceError for a client-generated failure. Typing `code` as the +// closed `ClientErrorCode` (rather than the open `WebServiceErrorCode` the +// WebServiceError constructor accepts) makes a typo at a throw site a compile +// error and keeps the `ClientErrorCode` union in sync with what the client +// actually emits. +const clientError = ( + properties: { + code: ClientErrorCode; + error: string; + status?: number; + url: string; + }, + options?: { cause?: unknown } +): WebServiceError => new WebServiceError(properties, options); const isErrorBody = (data: unknown): data is { code: string; error: string } => typeof data === 'object' && @@ -110,7 +126,7 @@ export default class WebServiceClient { ? err : new Error(String(err)); if (error.name === 'TimeoutError') { - throw new WebServiceError( + throw clientError( { code: 'NETWORK_TIMEOUT', error: 'The request timed out', @@ -124,7 +140,7 @@ export default class WebServiceClient { // only log `code`/`error`, not just available via `cause`. const causeDetail = error.cause instanceof Error ? `: ${error.cause.message}` : ''; - throw new WebServiceError( + throw clientError( { code: 'FETCH_ERROR', error: `${error.name} - ${error.message}${causeDetail}`, @@ -146,10 +162,7 @@ export default class WebServiceClient { try { data = await response.json(); } catch (err) { - throw new WebServiceError( - { ...invalidResponseBody, url }, - { cause: err } - ); + throw clientError({ ...invalidResponseBody, url }, { cause: err }); } return new modelClass(data); @@ -162,7 +175,7 @@ export default class WebServiceClient { const status = response.status; if (status && status >= 500 && status < 600) { - return new WebServiceError({ + return clientError({ code: 'SERVER_ERROR', error: `Received a server error with HTTP status code: ${status}`, status, @@ -171,7 +184,7 @@ export default class WebServiceClient { } if (status && (status < 400 || status >= 600)) { - return new WebServiceError({ + return clientError({ code: 'HTTP_STATUS_CODE_ERROR', error: `Received an unexpected HTTP status code: ${status}`, status, @@ -183,14 +196,14 @@ export default class WebServiceClient { try { data = await response.json(); } catch (err) { - return new WebServiceError( + return clientError( { ...invalidResponseBody, status, url }, { cause: err } ); } if (!isErrorBody(data)) { - return new WebServiceError({ ...invalidResponseBody, status, url }); + return clientError({ ...invalidResponseBody, status, url }); } return new WebServiceError({ From dc38a9947eb0b7210d4a93c2866e883bac57e377 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 17:29:46 +0000 Subject: [PATCH 2/9] Preserve underlying cause on ArgumentError Give ArgumentError an optional `cause` option and forward it to the Error constructor, matching WebServiceError and the geoip2-node error classes. This is additive; no current caller passes a cause. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/errors.spec.ts | 11 +++++++++++ src/errors.ts | 4 ++-- 2 files changed, 13 insertions(+), 2 deletions(-) diff --git a/src/errors.spec.ts b/src/errors.spec.ts index 5fdefcc1..fd9909f8 100644 --- a/src/errors.spec.ts +++ b/src/errors.spec.ts @@ -102,4 +102,15 @@ describe('ArgumentError', () => { expect(err.name).toBe('ArgumentError'); expect(err.message).toBe('bad input'); }); + + it('preserves the underlying cause when provided', () => { + const cause = new RangeError('bad value'); + const err = new ArgumentError('bad input', { cause }); + + expect(err.cause).toBe(cause); + }); + + it('leaves cause undefined when not provided', () => { + expect(new ArgumentError('bad input').cause).toBeUndefined(); + }); }); diff --git a/src/errors.ts b/src/errors.ts index f610e2ce..9e7e017c 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -2,8 +2,8 @@ import { WebServiceClientError, WebServiceErrorCode } from './types.js'; /* tslint:disable:max-classes-per-file */ export class ArgumentError extends Error { - constructor(message: string) { - super(message); + constructor(message: string, options?: { cause?: unknown }) { + super(message, options); this.name = this.constructor.name; } } From e4d6959478e17e86e18145a3859eff2e4269ba8f Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 17:31:59 +0000 Subject: [PATCH 3/9] Accept an options object and an injectable fetcher in the client Change the WebServiceClient constructor from `(accountID, licenseKey, timeout?, host?)` to `(accountID, licenseKey, options?: Options | number)`, matching geoip2-node. `Options` carries `timeout`, `host`, and a new `fetcher` (a custom `fetch` implementation, useful for proxies/dispatchers and testing). A bare number is still accepted as the timeout for backward compatibility, so existing `new Client(id, key, 3000)` callers are unaffected; only the rare caller that passed `host` as a fourth positional argument must switch to `{ host }`. Requests now go through `this.fetcher` rather than the global `fetch`. Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 12 ++++-- src/index.ts | 1 + src/webServiceClient.spec.ts | 42 +++++++++++++++++++++ src/webServiceClient.ts | 71 ++++++++++++++++++++++++++++++++---- 4 files changed, 115 insertions(+), 11 deletions(-) diff --git a/README.md b/README.md index 585dd640..e8f33e02 100644 --- a/README.md +++ b/README.md @@ -34,13 +34,19 @@ takes your MaxMind account ID and license key. For example: const client = new minFraud.Client("1234", "LICENSEKEY"); ``` -If you would like to use the Sandbox environment, you can -set the `host` parameter to `sandbox.maxmind.com`: +The constructor also takes an optional third argument: an options object with +`timeout` (milliseconds, default `3000`), `host` (default `minfraud.maxmind.com`), +and `fetcher` (a custom `fetch` implementation, e.g. to route requests through a +proxy or custom dispatcher). If you would like to use the Sandbox environment, +set `host` to `sandbox.maxmind.com`: ```js -const client = new minFraud.Client("1234", "LICENSEKEY", 3000, 'sandbox.maxmind.com'); +const client = new minFraud.Client("1234", "LICENSEKEY", { host: 'sandbox.maxmind.com' }); ``` +For backward compatibility, a number may be passed as the third argument and is +treated as the `timeout`, though this is deprecated. + Then create a new `Transaction` object. This represents the transaction that you are sending to minFraud. Each transaction property is instantiated by creating a new instance of each property's class. For example: diff --git a/src/index.ts b/src/index.ts index e7fab990..993522bb 100644 --- a/src/index.ts +++ b/src/index.ts @@ -40,3 +40,4 @@ export type { WebServiceClientError, WebServiceErrorCode, } from './types.js'; +export type { WebServiceClientOptions } from './webServiceClient.js'; diff --git a/src/webServiceClient.spec.ts b/src/webServiceClient.spec.ts index 4b8b231b..0c15bb1a 100644 --- a/src/webServiceClient.spec.ts +++ b/src/webServiceClient.spec.ts @@ -5,6 +5,7 @@ import reasons from '../fixtures/reasons.json' with { type: 'json' }; import score from '../fixtures/score.json' with { type: 'json' }; import subscores from '../fixtures/subscores.json' with { type: 'json' }; import { + ArgumentError, Client, Constants, Device, @@ -35,6 +36,47 @@ describe('WebServiceClient', () => { factors.response.full.risk_score_reasons = structuredClone(reasons); factors.response.full.subscores = structuredClone(subscores); + describe('fetcher option', () => { + it('uses an injected fetcher instead of the global fetch', async () => { + const calls: { init?: RequestInit; url: RequestInfo | URL }[] = []; + const fetcher = ((url: RequestInfo | URL, init?: RequestInit) => { + calls.push({ init, url }); + return Promise.resolve( + new Response(JSON.stringify(score.response.full), { + headers: { 'content-type': 'application/json' }, + status: 200, + }) + ); + }) as typeof fetch; + const localClient = new Client(auth.user, auth.pass, { fetcher }); + const transaction = new Transaction({ + device: new Device({ ipAddress: '1.1.1.1' }), + }); + + const got = await localClient.score(transaction); + + expect(calls).toHaveLength(1); + expect(calls[0].url).toBe(`${baseUrl}${fullPath('score')}`); + expect(got.riskScore).toEqual(0.01); + }); + + it('treats a null options argument like no options', () => { + // A JS caller may pass an explicit null; it must not crash the + // constructor (typeof null === 'object'). + expect( + () => new Client(auth.user, auth.pass, null as unknown as undefined) + ).not.toThrow(); + }); + + it('rejects a legacy positional host argument', () => { + // The old constructor took (accountID, licenseKey, timeout, host); a + // fourth argument now indicates an out-of-date call site. + expect( + () => new Client(auth.user, auth.pass, 3000, 'proxy.example' as never) + ).toThrow(ArgumentError); + }); + }); + describe('factors()', () => { const transaction = new Transaction({ device: new Device({ diff --git a/src/webServiceClient.ts b/src/webServiceClient.ts index d1d4c468..4f383431 100644 --- a/src/webServiceClient.ts +++ b/src/webServiceClient.ts @@ -1,10 +1,23 @@ import packageInfo from '../package.json' with { type: 'json' }; -import { WebServiceError } from './errors.js'; +import { ArgumentError, WebServiceError } from './errors.js'; import Transaction from './request/transaction.js'; import TransactionReport from './request/transaction-report.js'; import * as models from './response/models/index.js'; import { ClientErrorCode } from './types.js'; +/** Options for the WebServiceClient constructor */ +export interface WebServiceClientOptions { + /** A custom `fetch` implementation to use for requests. Defaults to the + * global `fetch`. This is primarily useful for testing or for routing + * requests through a custom dispatcher or proxy. */ + fetcher?: typeof fetch; + /** The host to use when connecting to the web service. Defaults to + * "minfraud.maxmind.com". */ + host?: string; + /** The timeout in milliseconds. Defaults to 3000. */ + timeout?: number; +} + type servicePath = 'factors' | 'insights' | 'score' | 'transactions/report'; const invalidResponseBody = { @@ -37,20 +50,62 @@ const isErrorBody = (data: unknown): data is { code: string; error: string } => export default class WebServiceClient { private accountID: string; - private host: string; private licenseKey: string; - private timeout: number; + private host = 'minfraud.maxmind.com'; + private timeout = 3000; + private fetcher: typeof fetch = fetch; + /** + * Instantiates a WebServiceClient. + * + * @param accountID The account ID + * @param licenseKey The license key + * @param options Additional options. If you pass a number as the third + * parameter, it will be treated as the timeout; however, + * passing in a number should be considered deprecated and may + * be removed in a future major version. + */ public constructor( accountID: string, licenseKey: string, - timeout = 3000, - host = 'minfraud.maxmind.com' + // We support a number, which will be treated as the timeout for historical + // reasons. + options?: WebServiceClientOptions | number, + // The constructor previously took a positional `host` here. Reject a fourth + // argument loudly rather than silently ignoring it (which would route + // requests to the default host). + legacyHost?: never ) { this.accountID = accountID; this.licenseKey = licenseKey; - this.timeout = timeout; - this.host = host; + if (legacyHost !== undefined) { + throw new ArgumentError( + 'The WebServiceClient constructor no longer accepts a positional ' + + '`host` argument; pass `{ host }` in the options object instead.' + ); + } + // `typeof null === 'object'`, so guard null alongside undefined to avoid + // dereferencing it in the options branch below. + if (options === undefined || options === null) { + return; + } + + if (typeof options === 'object') { + if (options.fetcher !== undefined) { + this.fetcher = options.fetcher; + } + + if (options.host !== undefined) { + this.host = options.host; + } + + if (options.timeout !== undefined) { + this.timeout = options.timeout; + } + return; + } + + this.timeout = options; } public factors(transaction: Transaction): Promise { @@ -119,7 +174,7 @@ export default class WebServiceClient { */ let response: Response; try { - response = await fetch(url, options); + response = await this.fetcher(url, options); } catch (err) { const error = err instanceof Error || err instanceof DOMException From fc6473921196ea86eace10ba496b5f17a05f53af Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 17:33:33 +0000 Subject: [PATCH 4/9] Stop sanitizeKeys() from mutating caller-owned objects MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Transaction.toString() builds a shallow copy via Object.assign({}, this), so the nested billing/shipping/creditCard objects were still the caller's instances — and the address2 -> address_2 and was3DSecureSuccessful -> was_3d_secure_successful renames mutated them in place (a caller's Billing would lose address2 and gain address_2 after serialization). Rebuild each renamed object as a fresh object instead, leaving the caller's instances untouched. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/request/transaction.spec.ts | 41 +++++++++++++++++++++++++++++++++ src/request/transaction.ts | 35 +++++++++++++++++----------- 2 files changed, 62 insertions(+), 14 deletions(-) diff --git a/src/request/transaction.spec.ts b/src/request/transaction.spec.ts index eeeb75ab..1730630b 100644 --- a/src/request/transaction.spec.ts +++ b/src/request/transaction.spec.ts @@ -214,6 +214,47 @@ describe('Transaction()', () => { expect(test.toString()).toContain('"billing":{"address_2":"foo"}'); }); + it('does not mutate the caller’s input objects when serializing', () => { + const billing = new Billing({ address2: 'foo' }); + const shipping = new Shipping({ address2: 'bar' }); + const creditCard = new CreditCard({ was3DSecureSuccessful: true }); + const order = new Order({ + referrerUri: new URL('https://example.com/foo'), + }); + const test = new Transaction({ + billing, + creditCard, + device: new Device({ ipAddress: '1.1.1.1' }), + order, + shipping, + }); + + const serialized = test.toString(); + expect(serialized).toContain('"billing":{"address_2":"foo"}'); + expect(serialized).toContain('"shipping":{"address_2":"bar"}'); + expect(serialized).toContain('"was_3d_secure_successful":true'); + expect(serialized).toContain('"referrer_uri":"https://example.com/foo"'); + + // Serializing must not have rewritten any of the caller's instances. + expect(billing.address2).toEqual('foo'); + expect(Object.prototype.hasOwnProperty.call(billing, 'address_2')).toBe( + false + ); + expect(shipping.address2).toEqual('bar'); + expect(Object.prototype.hasOwnProperty.call(shipping, 'address_2')).toBe( + false + ); + expect(creditCard.was3DSecureSuccessful).toBe(true); + expect( + Object.prototype.hasOwnProperty.call( + creditCard, + 'was_3d_secure_successful' + ) + ).toBe(false); + // referrerUri must still be the original URL, not a stringified copy. + expect(order.referrerUri).toBeInstanceOf(URL); + }); + it('it handles optional shipping field', () => { const test = new Transaction({ device: new Device({ diff --git a/src/request/transaction.ts b/src/request/transaction.ts index 46d581a5..a92137f9 100644 --- a/src/request/transaction.ts +++ b/src/request/transaction.ts @@ -97,13 +97,7 @@ export default class Transaction { } public toString(): string { - const sanitized = this.sanitizeKeys(); - - if (sanitized.order != null && sanitized.order.referrerUri) { - sanitized.order.referrerUri = sanitized.order.referrerUri.toString(); - } - - return JSON.stringify(snakecaseKeys(sanitized)); + return JSON.stringify(snakecaseKeys(this.sanitizeKeys())); } // eslint-disable-next-line @typescript-eslint/no-explicit-any @@ -119,20 +113,24 @@ export default class Transaction { // eslint-disable-next-line @typescript-eslint/no-explicit-any const sanitized = Object.assign({}, this) as any; + // `Object.assign({}, this)` is a shallow copy, so the nested request + // objects are still the caller's instances. Rebuild each one we need to + // rename keys on as a fresh object rather than mutating it in place, so + // serializing a Transaction never mutates the objects the caller passed in. if ( sanitized.billing && Object.prototype.hasOwnProperty.call(sanitized.billing, 'address2') ) { - sanitized.billing.address_2 = sanitized.billing.address2; - delete sanitized.billing.address2; + const { address2, ...rest } = sanitized.billing; + sanitized.billing = { ...rest, address_2: address2 }; } if ( sanitized.shipping && Object.prototype.hasOwnProperty.call(sanitized.shipping, 'address2') ) { - sanitized.shipping.address_2 = sanitized.shipping.address2; - delete sanitized.shipping.address2; + const { address2, ...rest } = sanitized.shipping; + sanitized.shipping = { ...rest, address_2: address2 }; } if ( @@ -142,9 +140,18 @@ export default class Transaction { 'was3DSecureSuccessful' ) ) { - sanitized.creditCard.was_3d_secure_successful = - sanitized.creditCard.was3DSecureSuccessful; - delete sanitized.creditCard.was3DSecureSuccessful; + const { was3DSecureSuccessful, ...rest } = sanitized.creditCard; + sanitized.creditCard = { + ...rest, + was_3d_secure_successful: was3DSecureSuccessful, + }; + } + + if (sanitized.order && sanitized.order.referrerUri) { + sanitized.order = { + ...sanitized.order, + referrerUri: sanitized.order.referrerUri.toString(), + }; } return sanitized; From 4d4dc3bddcb57245dcbc1ee353942ca99abc3920 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 17:35:10 +0000 Subject: [PATCH 5/9] Accept customInputs as a plain record TransactionProps.customInputs was typed CustomInput[] but the stored field is a Record, and the value was always run through Object.assign to flatten it. Additively widen the input type to `CustomInput[] | Record` and handle both: an array is flattened as before, a record is used directly. Existing CustomInput[] callers are unaffected. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/request/transaction.spec.ts | 40 +++++++++++++++++++++++++++++++++ src/request/transaction.ts | 34 +++++++++++++++++++++------- 2 files changed, 66 insertions(+), 8 deletions(-) diff --git a/src/request/transaction.spec.ts b/src/request/transaction.spec.ts index 1730630b..3b3c433b 100644 --- a/src/request/transaction.spec.ts +++ b/src/request/transaction.spec.ts @@ -119,6 +119,46 @@ describe('Transaction()', () => { }); }); + it('accepts custom inputs as a plain record', () => { + const test = new Transaction({ + customInputs: { fizz: 'buzz', foo: 'bar' }, + device: new Device({ + ipAddress: '1.1.1.1', + }), + }); + + expect(test.customInputs).toEqual({ + fizz: 'buzz', + foo: 'bar', + }); + }); + + it('throws if a customInputs record value is not a primitive', () => { + const test = () => + new Transaction({ + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + customInputs: { foo: { nested: true } }, + device: new Device({ + ipAddress: '1.1.1.1', + }), + }); + expect(test).toThrow(ArgumentError); + expect(test).toThrow('customInputs'); + }); + + it('throws if a customInputs record value is a non-finite number', () => { + const test = () => + new Transaction({ + customInputs: { foo: NaN }, + device: new Device({ + ipAddress: '1.1.1.1', + }), + }); + expect(test).toThrow(ArgumentError); + expect(test).toThrow('customInputs'); + }); + describe('toString()', () => { const deviceString = '"device":{"ip_address":"1.1.1.1","session_age":100}'; diff --git a/src/request/transaction.ts b/src/request/transaction.ts index a92137f9..cb9b3e57 100644 --- a/src/request/transaction.ts +++ b/src/request/transaction.ts @@ -26,9 +26,11 @@ export interface TransactionProps { */ creditCard?: CreditCard; /** - * Custom inputs as configured on your account portal. + * Custom inputs as configured on your account portal. This may be provided + * either as an array of `CustomInput` instances or as a plain object mapping + * input keys to their values. */ - customInputs?: CustomInput[]; + customInputs?: CustomInput[] | Record; /** * Information about the device used in the transaction. */ @@ -92,7 +94,9 @@ export default class Transaction { Object.assign(this, transaction); if (transaction.customInputs != null) { - this.customInputs = Object.assign({}, ...transaction.customInputs); + this.customInputs = Array.isArray(transaction.customInputs) + ? Object.assign({}, ...transaction.customInputs) + : { ...transaction.customInputs }; } } @@ -190,11 +194,25 @@ export default class Transaction { } if (props.customInputs != null) { - for (const [idx, item] of props.customInputs.entries()) { - if (!(item instanceof CustomInput)) { - throw new ArgumentError( - `\`customInputs[${idx}]\` needs to be an instance of CustomInput` - ); + if (Array.isArray(props.customInputs)) { + for (const [idx, item] of props.customInputs.entries()) { + if (!(item instanceof CustomInput)) { + throw new ArgumentError( + `\`customInputs[${idx}]\` needs to be an instance of CustomInput` + ); + } + } + } else { + for (const [key, value] of Object.entries(props.customInputs)) { + const isValid = + typeof value === 'boolean' || + typeof value === 'string' || + (typeof value === 'number' && Number.isFinite(value)); + if (!isValid) { + throw new ArgumentError( + `\`customInputs.${key}\` needs to be a boolean, finite number, or string` + ); + } } } } From 08bebe2613ccb594137bc6349b45c5dbb32ca985 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 19:34:41 +0000 Subject: [PATCH 6/9] Rename camelizeResponse to camelcaseKeys Match geoip2-node's utility name for cross-library consistency. This is an internal helper (not exported from the package), so the rename has no effect on consumers. Co-Authored-By: Claude Opus 4.8 (1M context) --- src/response/models/factors.ts | 4 ++-- src/response/models/insights.ts | 4 ++-- src/response/models/score.ts | 6 +++--- src/utils.spec.ts | 16 ++++++++++++---- src/utils.ts | 13 +++++++------ 5 files changed, 26 insertions(+), 17 deletions(-) diff --git a/src/response/models/factors.ts b/src/response/models/factors.ts index 5a3b8244..67ed2992 100644 --- a/src/response/models/factors.ts +++ b/src/response/models/factors.ts @@ -1,4 +1,4 @@ -import { camelizeResponse } from '../../utils.js'; +import { camelcaseKeys } from '../../utils.js'; import * as records from '../records.js'; import * as webRecords from '../web-records.js'; import Insights from './insights.js'; @@ -22,6 +22,6 @@ export default class Factors extends Insights { super(response); this.riskScoreReasons = response.risk_score_reasons; - this.subscores = camelizeResponse(response.subscores) as records.Subscores; + this.subscores = camelcaseKeys(response.subscores) as records.Subscores; } } diff --git a/src/response/models/insights.ts b/src/response/models/insights.ts index c267b77b..b479ad69 100644 --- a/src/response/models/insights.ts +++ b/src/response/models/insights.ts @@ -1,5 +1,5 @@ import { Insights as GeoInsights } from '@maxmind/geoip2-node'; -import { camelizeResponse } from '../../utils.js'; +import { camelcaseKeys } from '../../utils.js'; import * as records from '../records.js'; import * as webRecords from '../web-records.js'; import Score from './score.js'; @@ -73,7 +73,7 @@ export default class Insights extends Score { response: webRecords.InsightsResponse, prop: keyof webRecords.InsightsResponse ): T | undefined { - return response[prop] ? (camelizeResponse(response[prop]) as T) : undefined; + return response[prop] ? (camelcaseKeys(response[prop]) as T) : undefined; } private getIpAddress( diff --git a/src/response/models/score.ts b/src/response/models/score.ts index edbf7b5a..71de18d8 100644 --- a/src/response/models/score.ts +++ b/src/response/models/score.ts @@ -1,4 +1,4 @@ -import { camelizeResponse } from '../../utils.js'; +import { camelcaseKeys } from '../../utils.js'; import * as records from '../records.js'; import * as webRecords from '../web-records.js'; @@ -42,7 +42,7 @@ export default class Score { public readonly warnings?: records.Warning[]; public constructor(response: webRecords.ScoreResponse) { - this.disposition = camelizeResponse( + this.disposition = camelcaseKeys( response.disposition ) as records.Disposition; this.fundsRemaining = response.funds_remaining; @@ -51,7 +51,7 @@ export default class Score { this.queriesRemaining = response.queries_remaining; this.riskScore = response.risk_score; this.warnings = response.warnings - ? (camelizeResponse(response.warnings) as records.Warning[]) + ? (camelcaseKeys(response.warnings) as records.Warning[]) : undefined; } } diff --git a/src/utils.spec.ts b/src/utils.spec.ts index bf8e17cb..1b901a29 100644 --- a/src/utils.spec.ts +++ b/src/utils.spec.ts @@ -1,4 +1,4 @@ -import { snakecaseKeys, snakeToCamelCase, camelizeResponse } from './utils.js'; +import { snakecaseKeys, snakeToCamelCase, camelcaseKeys } from './utils.js'; describe('src/Utils', () => { describe('snakeToCamelCase()', () => { @@ -59,6 +59,14 @@ describe('src/Utils', () => { }); }); describe('camelcaseKeys()', () => { + it('returns non-object input unchanged', () => { + expect(camelcaseKeys('foo')).toBe('foo'); + expect(camelcaseKeys(42)).toBe(42); + expect(camelcaseKeys(true)).toBe(true); + expect(camelcaseKeys(null)).toBeNull(); + expect(camelcaseKeys(undefined)).toBeUndefined(); + }); + it("converts an object's keys from snake_case to camelCase", () => { const cases = [ { input: { snake_case: 1 }, expected: { snakeCase: 1 } }, @@ -68,7 +76,7 @@ describe('src/Utils', () => { }, ]; cases.forEach((testCase) => { - expect(camelizeResponse(testCase.input)).toEqual(testCase.expected); + expect(camelcaseKeys(testCase.input)).toEqual(testCase.expected); }); }); @@ -96,7 +104,7 @@ describe('src/Utils', () => { }, ]; cases.forEach((testCase) => { - expect(camelizeResponse(testCase.input)).toEqual(testCase.expected); + expect(camelcaseKeys(testCase.input)).toEqual(testCase.expected); }); }); @@ -142,7 +150,7 @@ describe('src/Utils', () => { }, ]; cases.forEach((testCase) => { - expect(camelizeResponse(testCase.input)).toEqual(testCase.expected); + expect(camelcaseKeys(testCase.input)).toEqual(testCase.expected); }); }); }); diff --git a/src/utils.ts b/src/utils.ts index af2a28a9..ebd186e0 100644 --- a/src/utils.ts +++ b/src/utils.ts @@ -22,7 +22,7 @@ const processArray = (arr: Array): unknown[] => Array.isArray(el) ? processArray(el) : isObject(el) - ? camelizeResponse(el as Record) + ? camelcaseKeys(el as Record) : el ); @@ -31,13 +31,14 @@ const processArray = (arr: Array): unknown[] => * @param input - object with some snake_case keys * @returns - object with camelCase keys */ -export function camelizeResponse(input: unknown): unknown { - if (!input) { - return input; - } +export function camelcaseKeys(input: unknown): unknown { if (Array.isArray(input)) { return processArray(input); } + // Leave primitives (and null/undefined) untouched, matching snakecaseKeys. + if (!isObject(input)) { + return input; + } const output: Record = {}; @@ -45,7 +46,7 @@ export function camelizeResponse(input: unknown): unknown { if (Array.isArray(value)) { output[snakeToCamelCase(key)] = processArray(value); } else if (isObject(value)) { - output[snakeToCamelCase(key)] = camelizeResponse( + output[snakeToCamelCase(key)] = camelcaseKeys( value as Record ); } else { From 62363bdcdf1e7423b31977319a88408936a7935c Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 19:38:20 +0000 Subject: [PATCH 7/9] Remove the validator production dependency Replace the three `validator` uses with built-ins: * email.ts: reimplement the `isEmail`/`isFQDN` input guards with small regex helpers (covered by the existing email.spec.ts cases, including IDN domains and trailing-dot rejections). * order.ts: the `referrerUri` `isURL` check was effectively validating a value that is already a `URL`; validate with the native `URL` constructor instead, preserving the ArgumentError on invalid input. * transaction.spec.ts: replace the test-only `isJSON` with a local `JSON.parse` helper. Drops `validator` and `@types/validator` from the dependencies. Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 19 +++++++++------ lychee.toml | 2 ++ package-lock.json | 20 +--------------- package.json | 4 +--- src/request/email.spec.ts | 42 +++++++++++++++++++++++++++++++++ src/request/email.ts | 40 ++++++++++++++++++++++++++++--- src/request/order.spec.ts | 18 ++++++++++++++ src/request/order.ts | 29 ++++++++++++++++------- src/request/transaction.spec.ts | 36 +++++++++++++++++----------- 9 files changed, 156 insertions(+), 54 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6e83e9ef..9e43cda4 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -57,7 +57,7 @@ Responses use snake_case and are converted to camelCase in model constructors: // Model exposes: response.riskScore, response.fundsRemaining ``` -The `camelizeResponse()` utility in `utils.ts` handles deep conversion for response data. +The `camelcaseKeys()` utility in `utils.ts` handles deep conversion for response data. The `snakecaseKeys()` utility converts request objects to snake_case. #### 2. **Model Inheritance Hierarchy** @@ -293,12 +293,12 @@ Always update `CHANGELOG.md` for user-facing changes. API responses use snake_case but must be exposed as camelCase. -**Solution**: Use `camelizeResponse()` for nested objects: +**Solution**: Use `camelcaseKeys()` for nested objects: ```typescript this.email = this.maybeGet(response, 'email'); private maybeGet(response: Response, prop: keyof Response): T | undefined { - return response[prop] ? (camelizeResponse(response[prop]) as T) : undefined; + return response[prop] ? (camelcaseKeys(response[prop]) as T) : undefined; } ``` @@ -306,15 +306,20 @@ private maybeGet(response: Response, prop: keyof Response): T | undefined { Request components should validate inputs in constructors. -**Solution**: Import validators from the `validator` package: +**Solution**: Validate inputs in the constructor and throw `ArgumentError` on +failure, using built-ins (the library has no validation dependency): + ```typescript -import validator from 'validator'; +import { isIP } from 'node:net'; -if (!validator.isEmail(props.address)) { - throw new ArgumentError('Invalid email address'); +if (props.ipAddress != null && isIP(props.ipAddress) === 0) { + throw new ArgumentError('Invalid IP address'); } ``` +Email and domain checks use small local regex helpers (`src/request/email.ts`), +and URLs are validated with the `URL` constructor (`src/request/order.ts`). + ### Problem: Missing Error Handling The client can throw various error types. diff --git a/lychee.toml b/lychee.toml index 8b71aa93..e4d31263 100644 --- a/lychee.toml +++ b/lychee.toml @@ -44,6 +44,8 @@ exclude = [ # Placeholders / example URLs used in tests and docstrings '^https?://example\.(com|org|net)', '^https?://(www\.)?foobar\.com', + # `http://foo` is a single-label host used to test referrer-URI validation + '^https?://foo/?$', '^http://google\.com', '^http://localhost', '127\.0\.0\.1', diff --git a/package-lock.json b/package-lock.json index a6fcafec..94144716 100644 --- a/package-lock.json +++ b/package-lock.json @@ -10,14 +10,12 @@ "license": "Apache-2.0", "dependencies": { "@maxmind/geoip2-node": "^6.3.0", - "maxmind": "^5.0.0", - "validator": "^13.0.0" + "maxmind": "^5.0.0" }, "devDependencies": { "@eslint/js": "^10.0.1", "@types/jest": "^30.0.0", "@types/node": "^25.0.3", - "@types/validator": "^13.0.0", "eslint": "^10.0.2", "eslint-config-prettier": "^10.0.1", "globals": "^17.0.0", @@ -1583,13 +1581,6 @@ "dev": true, "license": "MIT" }, - "node_modules/@types/validator": { - "version": "13.15.10", - "resolved": "https://registry.npmjs.org/@types/validator/-/validator-13.15.10.tgz", - "integrity": "sha512-T8L6i7wCuyoK8A/ZeLYt1+q0ty3Zb9+qbSSvrIVitzT3YjZqkTZ40IbRsPanlB4h1QB3JVL1SYCdR6ngtFYcuA==", - "dev": true, - "license": "MIT" - }, "node_modules/@types/yargs": { "version": "17.0.35", "resolved": "https://registry.npmjs.org/@types/yargs/-/yargs-17.0.35.tgz", @@ -5860,15 +5851,6 @@ "node": ">=10.12.0" } }, - "node_modules/validator": { - "version": "13.15.35", - "resolved": "https://registry.npmjs.org/validator/-/validator-13.15.35.tgz", - "integrity": "sha512-TQ5pAGhd5whStmqWvYF4OjQROlmv9SMFVt37qoCBdqRffuuklWYQlCNnEs2ZaIBD1kZRNnikiZOS1eqgkar0iw==", - "license": "MIT", - "engines": { - "node": ">= 0.10" - } - }, "node_modules/walker": { "version": "1.0.8", "resolved": "https://registry.npmjs.org/walker/-/walker-1.0.8.tgz", diff --git a/package.json b/package.json index 4ae8ef07..e6bfb206 100644 --- a/package.json +++ b/package.json @@ -36,7 +36,6 @@ "@eslint/js": "^10.0.1", "@types/jest": "^30.0.0", "@types/node": "^25.0.3", - "@types/validator": "^13.0.0", "eslint": "^10.0.2", "eslint-config-prettier": "^10.0.1", "globals": "^17.0.0", @@ -65,7 +64,6 @@ }, "dependencies": { "@maxmind/geoip2-node": "^6.3.0", - "maxmind": "^5.0.0", - "validator": "^13.0.0" + "maxmind": "^5.0.0" } } diff --git a/src/request/email.spec.ts b/src/request/email.spec.ts index 3ef8c6e9..12114c1b 100644 --- a/src/request/email.spec.ts +++ b/src/request/email.spec.ts @@ -30,6 +30,20 @@ describe('Email()', () => { expect(email).toThrow('email.address'); }); + it.each([ + 'foo@bar', // no dot in the domain + 'foo bar@example.com', // whitespace + 'foo@@example.com', // multiple @ + 'foo@-example.com', // label starting with a hyphen + 'foo@example..com', // empty label + 'foo@example.123', // purely numeric TLD + 'foo@1.2.3.4', // looks like an IP address + ])('email.address %p is not valid', (address) => { + const email = () => new Email({ address }); + expect(email).toThrow(ArgumentError); + expect(email).toThrow('email.address'); + }); + it('throws an error if email.domain is not valid', () => { const email = () => new Email({ @@ -39,6 +53,34 @@ describe('Email()', () => { expect(email).toThrow('email.domain'); }); + it('throws an ArgumentError (not a TypeError) for a non-string domain', () => { + const email = () => + new Email({ + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + domain: 123, + }); + expect(email).toThrow(ArgumentError); + expect(email).toThrow('email.domain'); + }); + + it('throws an error if a domain label exceeds 63 characters', () => { + const email = () => + new Email({ + domain: `${'a'.repeat(64)}.com`, + }); + expect(email).toThrow(ArgumentError); + expect(email).toThrow('email.domain'); + }); + + it('accepts a domain label of exactly 63 characters', () => { + expect(() => { + new Email({ + domain: `${'a'.repeat(63)}.com`, + }); + }).not.toThrow(); + }); + it('constructs', () => { expect(() => { new Email({ diff --git a/src/request/email.ts b/src/request/email.ts index 388d7c63..50710468 100644 --- a/src/request/email.ts +++ b/src/request/email.ts @@ -1,8 +1,42 @@ import crypto from 'node:crypto'; import { domainToASCII } from 'node:url'; -import validator from 'validator'; import { ArgumentError } from '../errors.js'; +// A fully-qualified domain name: two or more dot-separated labels of letters +// (including non-ASCII), combining marks, digits, or hyphens, with no label +// starting or ending with a hyphen. Replaces validator.isFQDN. +const isFQDN = (value: unknown): boolean => { + // Fail closed for non-string input so the constructor surfaces an + // ArgumentError rather than a raw TypeError from `.split`. + if (typeof value !== 'string' || value.length > 253) { + return false; + } + const labels = value.split('.'); + if (labels.length < 2) { + return false; + } + // The top-level domain must not be purely numeric (RFC 3696), which also + // rejects IP-address-like values. + if (/^\d+$/.test(labels[labels.length - 1])) { + return false; + } + return labels.every( + (label) => + label.length <= 63 && + /^[\p{L}\p{M}\p{N}-]+$/u.test(label) && + !label.startsWith('-') && + !label.endsWith('-') + ); +}; + +// A pragmatic email check: a non-empty local part with no whitespace or `@`, +// followed by a fully-qualified domain. This intentionally does not implement +// the full RFC 5322 grammar. Replaces validator.isEmail. +const isEmail = (value: string): boolean => { + const match = /^([^\s@]+)@([^\s@]+)$/.exec(value); + return match !== null && isFQDN(match[2]); +}; + interface EmailProps { /** * The email address used in the transaction. @@ -283,11 +317,11 @@ export default class Email implements EmailProps { }; public constructor(email: EmailProps) { - if (email.address != null && !validator.isEmail(email.address)) { + if (email.address != null && !isEmail(email.address)) { throw new ArgumentError('`email.address` is an invalid email address'); } - if (email.domain != null && !validator.isFQDN(email.domain)) { + if (email.domain != null && !isFQDN(email.domain)) { throw new ArgumentError('`email.domain` is an invalid domain'); } diff --git a/src/request/order.spec.ts b/src/request/order.spec.ts index fe4b5c3b..3eb43362 100644 --- a/src/request/order.spec.ts +++ b/src/request/order.spec.ts @@ -22,6 +22,24 @@ describe('Order()', () => { expect(order).toThrow('referrer URI'); }); + it('throws an error if the referrer URI scheme is not http(s)', () => { + const order = () => + new Order({ + referrerUri: new URL('javascript:alert(1)'), + }); + expect(order).toThrow(ArgumentError); + expect(order).toThrow('referrer URI'); + }); + + it('throws an error if the referrer URI host has no dot', () => { + const order = () => + new Order({ + referrerUri: new URL('http://foo'), + }); + expect(order).toThrow(ArgumentError); + expect(order).toThrow('referrer URI'); + }); + it('constructs', () => { expect(() => { new Order({ diff --git a/src/request/order.ts b/src/request/order.ts index 1c89b1c2..be39776b 100644 --- a/src/request/order.ts +++ b/src/request/order.ts @@ -1,4 +1,3 @@ -import validator from 'validator'; import { ArgumentError } from '../errors.js'; interface OrderProps { @@ -65,13 +64,27 @@ export default class Order implements OrderProps { throw new ArgumentError(`The currency code ${order.currency} is invalid`); } - if ( - order.referrerUri != null && - !validator.isURL(order.referrerUri.toString()) - ) { - throw new ArgumentError( - `The referrer URI ${order.referrerUri.toString()} is invalid` - ); + if (order.referrerUri != null) { + let parsed: URL; + try { + // The URL constructor throws for non-absolute or otherwise invalid URLs. + parsed = new URL(order.referrerUri.toString()); + } catch { + throw new ArgumentError( + `The referrer URI ${order.referrerUri.toString()} is invalid` + ); + } + // Only http(s) referrers are meaningful; reject other schemes (e.g. + // javascript:, data:, mailto:) and single-label hosts (e.g. http://foo) + // the way the former validator.isURL did by default. + if ( + (parsed.protocol !== 'http:' && parsed.protocol !== 'https:') || + !parsed.hostname.includes('.') + ) { + throw new ArgumentError( + `The referrer URI ${order.referrerUri.toString()} is invalid` + ); + } } Object.assign(this, order); diff --git a/src/request/transaction.spec.ts b/src/request/transaction.spec.ts index 3b3c433b..0f195701 100644 --- a/src/request/transaction.spec.ts +++ b/src/request/transaction.spec.ts @@ -1,4 +1,3 @@ -import validator from 'validator'; import { ArgumentError } from '../errors.js'; import Account from './account.js'; import Billing from './billing.js'; @@ -13,6 +12,15 @@ import Shipping from './shipping.js'; import ShoppingCartItem from './shopping-cart-item.js'; import Transaction, { TransactionProps } from './transaction.js'; +const isJSON = (value: string): boolean => { + try { + JSON.parse(value); + return true; + } catch { + return false; + } +}; + describe('Transaction()', () => { it('does not throw an error if `device` is not defined', () => { const test = () => @@ -170,7 +178,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toEqual(`{${deviceString}}`); }); @@ -188,7 +196,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -209,7 +217,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -229,7 +237,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -247,7 +255,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -306,7 +314,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -324,7 +332,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -343,7 +351,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -364,7 +372,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -388,7 +396,7 @@ describe('Transaction()', () => { ], }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain(deviceString); @@ -538,7 +546,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain( '"credit_card":{"issuer_id_number":"12345678","last_digits":"12"}' @@ -557,7 +565,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain( '"credit_card":{"issuer_id_number":"12345678","last_digits":"1234"}' @@ -576,7 +584,7 @@ describe('Transaction()', () => { }), }); - expect(validator.isJSON(test.toString())).toBe(true); + expect(isJSON(test.toString())).toBe(true); expect(test.toString()).toContain( '"credit_card":{"issuer_id_number":"123456","last_digits":"12"}' From 6f4a43915549dbf77da9fd57ab21c55f08f0dd65 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 19:44:10 +0000 Subject: [PATCH 8/9] Replace nock with the injected fetcher in web service tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The WebServiceClient tests now drive requests through an injected `fetcher` that returns canned `Response`s and records the requests made, instead of mocking at the HTTP layer with nock. A dedicated test asserts the request shape (method, path, POST body, auth header) directly; error/status cases return the corresponding `Response`, and the timeout case rejects when the request signal aborts — exercising the real timeout signal deterministically. This removes the `nock` dev dependency (which was on a beta release). Co-Authored-By: Claude Opus 4.8 (1M context) --- CLAUDE.md | 20 ++-- package-lock.json | 97 ------------------ package.json | 1 - src/webServiceClient.spec.ts | 187 +++++++++++++++++------------------ 4 files changed, 104 insertions(+), 201 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 9e43cda4..77d78eeb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -182,18 +182,26 @@ Tests use `.spec.ts` files co-located with source: ### Test Patterns -Tests typically use `nock` to mock HTTP responses: -```typescript -import nock from 'nock'; +`WebServiceClient` tests inject a custom `fetcher` (via the constructor's +options object) rather than mocking at the HTTP layer. A small `clientWith()` +helper builds a client whose fetcher returns a canned `Response` and records the +requests made, so the request shape (method, path, body, auth header) and the +parsed response can both be asserted directly: -nock('https://minfraud.maxmind.com') - .post('/minfraud/v2.0/score') - .reply(200, { risk_score: 50, id: '...', /* ... */ }); +```typescript +const { client, requests } = clientWith(() => + jsonResponse(200, { risk_score: 50, id: '...', /* ... */ }) +); const response = await client.score(transaction); + +expect(requests[0].url).toBe('https://minfraud.maxmind.com/minfraud/v2.0/score'); expect(response.riskScore).toBe(50); ``` +Error and timeout cases return the appropriate `Response` (or a rejecting/ +signal-aware handler) from the fetcher. The library no longer depends on `nock`. + When adding new fields to models: 1. Update test fixtures/mocks to include the new field 2. Add assertions to verify the field is properly mapped diff --git a/package-lock.json b/package-lock.json index 94144716..7a033631 100644 --- a/package-lock.json +++ b/package-lock.json @@ -20,7 +20,6 @@ "eslint-config-prettier": "^10.0.1", "globals": "^17.0.0", "jest": "^30.0.0", - "nock": "^14.0.0-beta.16", "prettier": "^3.0.0", "ts-jest": "^29.4.0", "typedoc": "^0.28.1", @@ -1270,24 +1269,6 @@ "maxmind": "^5.0.0" } }, - "node_modules/@mswjs/interceptors": { - "version": "0.41.9", - "resolved": "https://registry.npmjs.org/@mswjs/interceptors/-/interceptors-0.41.9.tgz", - "integrity": "sha512-VVPPgHyQ6ShqnrmDWuxjmUIsO9gWyOZFmuOfLd9LfBGQJwZfy0gvv9pbHSJuoFNIYC7ZDX9aoFwowjcdSC4E8w==", - "dev": true, - "license": "MIT", - "dependencies": { - "@open-draft/deferred-promise": "^2.2.0", - "@open-draft/logger": "^0.3.0", - "@open-draft/until": "^2.0.0", - "is-node-process": "^1.2.0", - "outvariant": "^1.4.3", - "strict-event-emitter": "^0.5.1" - }, - "engines": { - "node": ">=18" - } - }, "node_modules/@napi-rs/wasm-runtime": { "version": "1.1.4", "resolved": "https://registry.npmjs.org/@napi-rs/wasm-runtime/-/wasm-runtime-1.1.4.tgz", @@ -1307,31 +1288,6 @@ "@emnapi/runtime": "^1.7.1" } }, - "node_modules/@open-draft/deferred-promise": { - "version": "2.2.0", - "resolved": "https://registry.npmjs.org/@open-draft/deferred-promise/-/deferred-promise-2.2.0.tgz", - "integrity": "sha512-CecwLWx3rhxVQF6V4bAgPS5t+So2sTbPgAzafKkVizyi7tlwpcFpdFqq+wqF2OwNBmqFuu6tOyouTuxgpMfzmA==", - "dev": true, - "license": "MIT" - }, - "node_modules/@open-draft/logger": { - "version": "0.3.0", - "resolved": "https://registry.npmjs.org/@open-draft/logger/-/logger-0.3.0.tgz", - "integrity": "sha512-X2g45fzhxH238HKO4xbSr7+wBS8Fvw6ixhTDuvLd5mqh6bJJCFAPwU9mPDxbcrRtfxv4u5IHCEH77BmxvXmmxQ==", - "dev": true, - "license": "MIT", - "dependencies": { - "is-node-process": "^1.2.0", - "outvariant": "^1.4.0" - } - }, - "node_modules/@open-draft/until": { - "version": "2.1.0", - "resolved": "https://registry.npmjs.org/@open-draft/until/-/until-2.1.0.tgz", - "integrity": "sha512-U69T3ItWHvLwGg5eJ0n3I62nWuE6ilHlmz7zM0npLBRvPRd7e6NYmg54vvRtP5mZG7kZqZCFVdsTWo7BPtBujg==", - "dev": true, - "license": "MIT" - }, "node_modules/@pkgjs/parseargs": { "version": "0.11.0", "resolved": "https://registry.npmjs.org/@pkgjs/parseargs/-/parseargs-0.11.0.tgz", @@ -3530,13 +3486,6 @@ "node": ">=0.10.0" } }, - "node_modules/is-node-process": { - "version": "1.2.0", - "resolved": "https://registry.npmjs.org/is-node-process/-/is-node-process-1.2.0.tgz", - "integrity": "sha512-Vg4o6/fqPxIjtxgUH5QLJhwZ7gW5diGCVlXpuUfELC62CuxM1iHcRe51f2W1FDy04Ai4KJkagKjx3XaqyfRKXw==", - "dev": true, - "license": "MIT" - }, "node_modules/is-stream": { "version": "2.0.1", "resolved": "https://registry.npmjs.org/is-stream/-/is-stream-2.0.1.tgz", @@ -4312,13 +4261,6 @@ "dev": true, "license": "MIT" }, - "node_modules/json-stringify-safe": { - "version": "5.0.1", - "resolved": "https://registry.npmjs.org/json-stringify-safe/-/json-stringify-safe-5.0.1.tgz", - "integrity": "sha512-ZClg6AaYvamvYEE82d3Iyd3vSSIjQ+odgjaTzRuO3s7toCdFKczob2i0zCh7JE8kWn17yvAWhUVxvqGwUalsRA==", - "dev": true, - "license": "ISC" - }, "node_modules/json5": { "version": "2.2.3", "resolved": "https://registry.npmjs.org/json5/-/json5-2.2.3.tgz", @@ -4635,21 +4577,6 @@ "dev": true, "license": "MIT" }, - "node_modules/nock": { - "version": "14.0.15", - "resolved": "https://registry.npmjs.org/nock/-/nock-14.0.15.tgz", - "integrity": "sha512-S0a47C9pLvcYx/Ugf0H30BVBEcUgMMBDk9VJIDlJ8XGrfH2QDUD4Tgdp45qDIiHttokBG+IbsOtsvIjGR/j3bg==", - "dev": true, - "license": "MIT", - "dependencies": { - "@mswjs/interceptors": "^0.41.0", - "json-stringify-safe": "^5.0.1", - "propagate": "^2.0.0" - }, - "engines": { - "node": ">=18.20.0 <20 || >=20.12.1" - } - }, "node_modules/node-int64": { "version": "0.4.0", "resolved": "https://registry.npmjs.org/node-int64/-/node-int64-0.4.0.tgz", @@ -4734,13 +4661,6 @@ "node": ">= 0.8.0" } }, - "node_modules/outvariant": { - "version": "1.4.3", - "resolved": "https://registry.npmjs.org/outvariant/-/outvariant-1.4.3.tgz", - "integrity": "sha512-+Sl2UErvtsoajRDKCE5/dBz4DIvHXQQnAxtQTF04OJxY0+DyZXSo5P5Bb7XYWOh81syohlYL24hbDwxedPUJCA==", - "dev": true, - "license": "MIT" - }, "node_modules/p-limit": { "version": "3.1.0", "resolved": "https://registry.npmjs.org/p-limit/-/p-limit-3.1.0.tgz", @@ -5017,16 +4937,6 @@ "url": "https://github.com/chalk/ansi-styles?sponsor=1" } }, - "node_modules/propagate": { - "version": "2.0.1", - "resolved": "https://registry.npmjs.org/propagate/-/propagate-2.0.1.tgz", - "integrity": "sha512-vGrhOavPSTz4QVNuBNdcNXePNdNMaO1xj9yBeH1ScQPjk/rhg9sSlCXPhMkFuaNNW/syTvYqsnbIJxMBfRbbag==", - "dev": true, - "license": "MIT", - "engines": { - "node": ">= 8" - } - }, "node_modules/punycode": { "version": "2.3.1", "resolved": "https://registry.npmjs.org/punycode/-/punycode-2.3.1.tgz", @@ -5220,13 +5130,6 @@ "node": ">=8" } }, - "node_modules/strict-event-emitter": { - "version": "0.5.1", - "resolved": "https://registry.npmjs.org/strict-event-emitter/-/strict-event-emitter-0.5.1.tgz", - "integrity": "sha512-vMgjE/GGEPEFnhFub6pa4FmJBRBVOLpIII2hvCZ8Kzb7K0hlHo7mQv6xYrBvCL2LtAIBwFUK8wvuJgTVSQ5MFQ==", - "dev": true, - "license": "MIT" - }, "node_modules/string-length": { "version": "4.0.2", "resolved": "https://registry.npmjs.org/string-length/-/string-length-4.0.2.tgz", diff --git a/package.json b/package.json index e6bfb206..c8b54c71 100644 --- a/package.json +++ b/package.json @@ -40,7 +40,6 @@ "eslint-config-prettier": "^10.0.1", "globals": "^17.0.0", "jest": "^30.0.0", - "nock": "^14.0.0-beta.16", "prettier": "^3.0.0", "ts-jest": "^29.4.0", "typedoc": "^0.28.1", diff --git a/src/webServiceClient.spec.ts b/src/webServiceClient.spec.ts index 0c15bb1a..b6fb1aae 100644 --- a/src/webServiceClient.spec.ts +++ b/src/webServiceClient.spec.ts @@ -1,4 +1,3 @@ -import nock from 'nock'; import * as models from './response/models/index.js'; import insights from '../fixtures/insights.json' with { type: 'json' }; import reasons from '../fixtures/reasons.json' with { type: 'json' }; @@ -16,47 +15,67 @@ import { import * as webRecords from './response/web-records.js'; const baseUrl = 'https://minfraud.maxmind.com'; -const nockInstance = nock(baseUrl); const auth = { pass: 'foo', user: '123', }; const fullPath = (path: string) => `/minfraud/v2.0/${path}`; -const client = new Client(auth.user, auth.pass); +interface CapturedRequest { + init?: RequestInit; + url: RequestInfo | URL; +} -describe('WebServiceClient', () => { - afterEach(() => { - nock.cleanAll(); - nock.abortPendingRequests(); +const jsonResponse = (status: number, body: unknown): Response => + new Response(JSON.stringify(body), { + headers: { 'content-type': 'application/json' }, + status, }); +// Builds a client backed by an injected fetcher driven by `handler`, capturing +// the requests the client makes so they can be asserted on. This replaces +// HTTP-level mocking: the handler returns the `Response` (or rejects) for each +// request. +const clientWith = ( + handler: (request: CapturedRequest) => Response | Promise, + options?: { host?: string; timeout?: number } +) => { + const requests: CapturedRequest[] = []; + const fetcher = (async (url: RequestInfo | URL, init?: RequestInit) => { + const request = { init, url }; + requests.push(request); + return handler(request); + }) as typeof fetch; + const client = new Client(auth.user, auth.pass, { fetcher, ...options }); + return { client, requests }; +}; + +describe('WebServiceClient', () => { // eslint-disable-next-line @typescript-eslint/no-explicit-any const factors = structuredClone(insights) as any; factors.response.full.risk_score_reasons = structuredClone(reasons); factors.response.full.subscores = structuredClone(subscores); describe('fetcher option', () => { - it('uses an injected fetcher instead of the global fetch', async () => { - const calls: { init?: RequestInit; url: RequestInfo | URL }[] = []; - const fetcher = ((url: RequestInfo | URL, init?: RequestInit) => { - calls.push({ init, url }); - return Promise.resolve( - new Response(JSON.stringify(score.response.full), { - headers: { 'content-type': 'application/json' }, - status: 200, - }) - ); - }) as typeof fetch; - const localClient = new Client(auth.user, auth.pass, { fetcher }); + it('uses the injected fetcher with the correct method, path, body, and auth', async () => { + const { client, requests } = clientWith(() => + jsonResponse(200, score.response.full) + ); const transaction = new Transaction({ device: new Device({ ipAddress: '1.1.1.1' }), }); - const got = await localClient.score(transaction); + const got = await client.score(transaction); - expect(calls).toHaveLength(1); - expect(calls[0].url).toBe(`${baseUrl}${fullPath('score')}`); + expect(requests).toHaveLength(1); + expect(requests[0].url).toBe(`${baseUrl}${fullPath('score')}`); + expect(requests[0].init!.method).toBe('POST'); + expect(requests[0].init!.body).toBe(transaction.toString()); + const headers = requests[0].init!.headers as Record; + expect(headers.Authorization).toBe( + 'Basic ' + btoa(`${auth.user}:${auth.pass}`) + ); + expect(headers['User-Agent']).toMatch(/^minfraud-api-node\//); expect(got.riskScore).toEqual(0.01); }); @@ -87,10 +106,9 @@ describe('WebServiceClient', () => { it('handles "full" responses', async () => { expect.assertions(180); - nockInstance - .post(fullPath('factors'), factors.request.basic) - .basicAuth(auth) - .reply(200, factors.response.full); + const { client } = clientWith(() => + jsonResponse(200, factors.response.full) + ); const got: models.Factors = await client.factors(transaction); @@ -349,10 +367,9 @@ describe('WebServiceClient', () => { it('handles "full" responses', async () => { expect.assertions(155); - nockInstance - .post(fullPath('insights'), insights.request.basic) - .basicAuth(auth) - .reply(200, insights.response.full); + const { client } = clientWith(() => + jsonResponse(200, insights.response.full) + ); const got: models.Insights = await client.insights(transaction); @@ -581,10 +598,7 @@ describe('WebServiceClient', () => { const response = structuredClone(insights.response.full); delete response[property as keyof webRecords.InsightsResponse]; - nockInstance - .post(fullPath('insights'), insights.request.basic) - .basicAuth(auth) - .reply(200, response); + const { client } = clientWith(() => jsonResponse(200, response)); const got: models.Insights = await client.insights(transaction); @@ -825,10 +839,9 @@ describe('WebServiceClient', () => { it('handles "full" responses', async () => { expect.assertions(11); - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(200, score.response.full); + const { client } = clientWith(() => + jsonResponse(200, score.response.full) + ); const got: models.Score = await client.score(transaction); @@ -850,9 +863,9 @@ describe('WebServiceClient', () => { it('handles "no disposition" responses', async () => { expect.assertions(8); - nockInstance - .post(fullPath('score'), score.request.basic) - .reply(200, score.response.noDisposition); + const { client } = clientWith(() => + jsonResponse(200, score.response.noDisposition) + ); const got: models.Score = await client.score(transaction); @@ -871,9 +884,9 @@ describe('WebServiceClient', () => { it('handles "no disposition rule_label" responses', async () => { expect.assertions(10); - nockInstance - .post(fullPath('score'), score.request.basic) - .reply(200, score.response.noDispositionRuleLabel); + const { client } = clientWith(() => + jsonResponse(200, score.response.noDispositionRuleLabel) + ); const got: models.Score = await client.score(transaction); @@ -894,9 +907,9 @@ describe('WebServiceClient', () => { it('handles "no warnings" responses', async () => { expect.assertions(8); - nockInstance - .post(fullPath('score'), score.request.basic) - .reply(200, score.response.noWarnings); + const { client } = clientWith(() => + jsonResponse(200, score.response.noWarnings) + ); const got: models.Score = await client.score(transaction); @@ -920,9 +933,7 @@ describe('WebServiceClient', () => { expect.assertions(1); - nockInstance - .post(fullPath('transactions/report'), report.toString()) - .reply(204); + const { client } = clientWith(() => new Response(null, { status: 204 })); await expect(client.reportTransaction(report)).resolves.toBeUndefined(); }); @@ -940,9 +951,7 @@ describe('WebServiceClient', () => { expect.assertions(1); - nockInstance - .post(fullPath('transactions/report'), report.toString()) - .reply(204); + const { client } = clientWith(() => new Response(null, { status: 204 })); await expect(client.reportTransaction(report)).resolves.toBeUndefined(); }); @@ -992,16 +1001,21 @@ describe('WebServiceClient', () => { }; it('handles timeouts', async () => { - const timeoutClient = new Client(auth.user, auth.pass, 10); - - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .delay(100) - .reply(200, score.response.full); + // The handler rejects only when the request signal aborts, which the + // client's timeout triggers — exercising the real timeout signal and the + // NETWORK_TIMEOUT mapping without a wall-clock delay. + const { client } = clientWith( + (request) => + new Promise((_resolve, reject) => { + request.init?.signal?.addEventListener('abort', () => + reject((request.init!.signal as AbortSignal).reason) + ); + }), + { timeout: 10 } + ); // The underlying abort/timeout error is preserved as the cause. - await expectError(timeoutClient.score(transaction), { + await expectError(client.score(transaction), { code: 'NETWORK_TIMEOUT', error: 'The request timed out', url: baseUrl + fullPath('score'), @@ -1010,10 +1024,7 @@ describe('WebServiceClient', () => { }); it('handles 5xx level errors', async () => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(500); + const { client } = clientWith(() => new Response(null, { status: 500 })); await expectError(client.score(transaction), { code: 'SERVER_ERROR', @@ -1025,10 +1036,7 @@ describe('WebServiceClient', () => { }); it('handles 3xx level errors', async () => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(300); + const { client } = clientWith(() => new Response(null, { status: 300 })); await expectError(client.score(transaction), { code: 'HTTP_STATUS_CODE_ERROR', @@ -1040,10 +1048,7 @@ describe('WebServiceClient', () => { }); it('handles errors with unknown payload', async () => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(401, { foo: 'bar' }); + const { client } = clientWith(() => jsonResponse(401, { foo: 'bar' })); await expectError(client.score(transaction), { code: 'INVALID_RESPONSE_BODY', @@ -1062,10 +1067,7 @@ describe('WebServiceClient', () => { `( 'treats $description as an invalid response body', async ({ payload }) => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(400, payload); + const { client } = clientWith(() => jsonResponse(400, payload)); await expectError(client.score(transaction), { code: 'INVALID_RESPONSE_BODY', @@ -1078,10 +1080,9 @@ describe('WebServiceClient', () => { ); it('preserves the cause for invalid response bodies', async () => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(200, 'this is not json'); + const { client } = clientWith( + () => new Response('this is not json', { status: 200 }) + ); const err = await expectError(client.score(transaction), { code: 'INVALID_RESPONSE_BODY', @@ -1096,10 +1097,9 @@ describe('WebServiceClient', () => { }); it('preserves the cause when an error response body is not JSON', async () => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(401, 'this is not json'); + const { client } = clientWith( + () => new Response('this is not json', { status: 401 }) + ); const err = await expectError(client.score(transaction), { code: 'INVALID_RESPONSE_BODY', @@ -1115,10 +1115,7 @@ describe('WebServiceClient', () => { it('handles general fetch errors', async () => { const error = 'general error'; - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .replyWithError(error); + const { client } = clientWith(() => Promise.reject(new Error(error))); const err = await expectError(client.score(transaction), { code: 'FETCH_ERROR', @@ -1136,10 +1133,7 @@ describe('WebServiceClient', () => { cause: new Error('connect ECONNREFUSED 1.2.3.4:443'), }); - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .replyWithError(fetchError); + const { client } = clientWith(() => Promise.reject(fetchError)); const err = await expectError(client.score(transaction), { code: 'FETCH_ERROR', @@ -1163,10 +1157,9 @@ describe('WebServiceClient', () => { ${402} | ${'INSUFFICIENT_FUNDS'} | ${'no money!'} ${403} | ${'PERMISSION_REQUIRED'} | ${'permission required'} `('handles $code error', async ({ code, error, status }) => { - nockInstance - .post(fullPath('score'), score.request.basic) - .basicAuth(auth) - .reply(status, { code, error }); + const { client } = clientWith(() => + jsonResponse(status, { code, error }) + ); await expectError(client.score(transaction), { code, From 3b3e9f8b950059a68c18dddcee0d532a10e39fb9 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Thu, 25 Jun 2026 19:45:29 +0000 Subject: [PATCH 9/9] Update CHANGELOG for the pre-major-release cleanups Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 25 ++++++++++++++++++++++++- 1 file changed, 24 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c6e13198..3004c1d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,7 +1,7 @@ CHANGELOG ========= -9.0.0 +9.0.0 (unreleased) ------------------ * **Breaking** Dropped support for Node.js 18 and 20. The minimum supported version is @@ -15,6 +15,29 @@ CHANGELOG property (for example, the network error behind a `FETCH_ERROR`). The `WebServiceError` and `ArgumentError` classes and the `WebServiceClientError` type are now exported from the package. +* **Breaking** The `WebServiceClient` constructor now takes an options object as + its third argument (`{ timeout, host, fetcher }`) instead of positional + `timeout` and `host` arguments. For backward compatibility a number may still + be passed and is treated as the `timeout`, so only callers that passed `host` + positionally need to change (to `{ host }`). The new `fetcher` option accepts + a custom `fetch` implementation, useful for routing requests through a proxy + or custom dispatcher, or for testing. +* The `code` property on `WebServiceError` and the `WebServiceClientError` + interface is now typed as `WebServiceErrorCode` + (`ClientErrorCode | (string & {})`) instead of `string`, providing + autocompletion for the client-generated codes while still accepting any code + returned by the web service. The `ClientErrorCode` and `WebServiceErrorCode` + types are exported from the package. +* `ArgumentError` now accepts an optional `cause` and forwards it to `Error`. +* `customInputs` on `Transaction` may now be provided as a plain object mapping + keys to values, in addition to an array of `CustomInput` instances. +* Removed the `validator` dependency. Email and domain validation now use + built-in regex helpers, and the order referrer URI is validated with the + `URL` constructor (restricted to `http`/`https`). Validation of a few + uncommon email edge cases may differ slightly. +* Fixed a bug where serializing a `Transaction` (e.g. when sending a request) + mutated the `Billing`, `Shipping`, `CreditCard`, and `Order` objects passed + to it. * Added the input `/device/tracking_token`. This is the token generated by the [Device Tracking Add-on](https://dev.maxmind.com/minfraud/track-devices) for explicit device linking. You may provide this by providing