From 8453e6ac034f8af7021a3c6f9bf9836f02f54f4a Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 24 Jun 2026 15:11:50 +0000 Subject: [PATCH 1/2] Throw WebServiceError instances and preserve error causes The web service client rejected with plain objects, which meant errors were not Error instances and the underlying cause (for example, the network error behind a FETCH_ERROR) was discarded. Introduce a WebServiceError class that extends Error and carries the existing code/error/status/url properties as own enumerable properties, plus the standard cause. Capture the cause at every throw site. Export WebServiceError and the WebServiceClientError type. STF-803 Co-Authored-By: Claude Opus 4.8 (1M context) --- CHANGELOG.md | 8 ++ README.md | 14 +++- src/errors.spec.ts | 95 ++++++++++++++++++++++++ src/errors.ts | 61 +++++++++++++++ src/index.ts | 1 + src/types.ts | 5 ++ src/webServiceClient.spec.ts | 139 ++++++++++++++++++++++++++++------- src/webServiceClient.ts | 85 +++++++++++++-------- 8 files changed, 346 insertions(+), 62 deletions(-) create mode 100644 src/errors.spec.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index cb7af791..9c053540 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,14 @@ CHANGELOG * **Breaking** Dropped support for Node.js 18 and 20. Node.js 22 or greater is now * **Breaking** Dropped commonjs support. The package is now only available as an ES module. +* **Breaking** Errors from the `WebServiceClient` are now thrown as + `WebServiceError` instances, which extend `Error`, rather than as plain + objects. The `code`, `error`, `status`, and `url` properties are preserved, + so existing field access continues to work, but the thrown value is now an + `Error`. The original error is now preserved as the standard `cause` + property (for example, the network error behind a `FETCH_ERROR`). The + `WebServiceError` class and the `WebServiceClientError` type are now exported + from the package. 6.3.4 (2025-11-25) ------------------ diff --git a/README.md b/README.md index be3bf850..3342a41d 100644 --- a/README.md +++ b/README.md @@ -39,7 +39,8 @@ If the request succeeds, the function's Promise will resolve with the model for the end point you called. This model in turn contains multiple records, each of which represents part of the data returned by the web service. -If the request fails, the function's Promise will reject with an error object. +If the request fails, the function's Promise will reject with a +`WebServiceError` (see [Web Service Errors](#web-service-errors) below). See the [API documentation](https://maxmind.github.io/GeoIP2-node/) for more details. @@ -106,17 +107,24 @@ client.insights('142.1.1.1').then(response => { For details on the possible errors returned by the web service itself, [see the GeoIP web service documentation](https://dev.maxmind.com/geoip/docs/web-services/responses/#errors). -If the web service returns an explicit error document, the promise will be rejected -with the following object structure: +Failures reject with a `WebServiceError`, which extends the built-in `Error`. +It exposes the following properties: ```js { code: 'THE_ERROR_CODE', error: 'some human readable error', + status: 400, // the HTTP status code, when available url: 'https://geoip.maxmind.com...', + cause: , } ``` +`error` is also available as the standard `Error` `message`, and when the +failure was caused by another error (for example, a network failure), the +original error is available as `cause`. Both `WebServiceError` and the +`WebServiceClientError` type are exported from the package. + In addition to the possible errors returned by the web service, the following error codes are provided: diff --git a/src/errors.spec.ts b/src/errors.spec.ts new file mode 100644 index 00000000..183dc97d --- /dev/null +++ b/src/errors.spec.ts @@ -0,0 +1,95 @@ +import { WebServiceError } from './errors.js'; + +describe('WebServiceError', () => { + it('is an Error instance', () => { + const err = new WebServiceError({ + code: 'FETCH_ERROR', + error: 'something went wrong', + url: 'https://example.com', + }); + + expect(err).toBeInstanceOf(Error); + expect(err).toBeInstanceOf(WebServiceError); + expect(err.name).toBe('WebServiceError'); + }); + + it('exposes code, error, status, and url', () => { + const err = new WebServiceError({ + code: 'SERVER_ERROR', + error: 'boom', + status: 500, + url: 'https://example.com', + }); + + expect(err.code).toBe('SERVER_ERROR'); + expect(err.error).toBe('boom'); + expect(err.status).toBe(500); + expect(err.url).toBe('https://example.com'); + }); + + it('uses the error string as the message', () => { + const err = new WebServiceError({ + code: 'FETCH_ERROR', + error: 'the message', + url: 'https://example.com', + }); + + expect(err.message).toBe('the message'); + expect(err.error).toBe(err.message); + }); + + it('preserves the underlying cause', () => { + const cause = new TypeError('fetch failed'); + const err = new WebServiceError( + { + code: 'FETCH_ERROR', + error: 'TypeError - fetch failed', + url: 'https://example.com', + }, + { cause } + ); + + expect(err.cause).toBe(cause); + }); + + it('leaves cause undefined when not provided', () => { + const err = new WebServiceError({ + code: 'FETCH_ERROR', + error: 'something went wrong', + url: 'https://example.com', + }); + + expect(err.cause).toBeUndefined(); + expect(JSON.parse(JSON.stringify(err))).not.toHaveProperty('cause'); + }); + + it('omits status when not provided', () => { + const err = new WebServiceError({ + code: 'FETCH_ERROR', + error: 'something went wrong', + url: 'https://example.com', + }); + + expect(err.status).toBeUndefined(); + expect(Object.prototype.hasOwnProperty.call(err, 'status')).toBe(false); + }); + + it('retains code, error, status, and url as enumerable properties', () => { + const err = new WebServiceError({ + code: 'SERVER_ERROR', + error: 'boom', + status: 500, + url: 'https://example.com', + }); + + const serialized = JSON.parse(JSON.stringify(err)); + expect(serialized).toMatchObject({ + code: 'SERVER_ERROR', + error: 'boom', + status: 500, + url: 'https://example.com', + }); + // `name` lives on the prototype, so it is not serialized. + expect(serialized).not.toHaveProperty('name'); + }); +}); diff --git a/src/errors.ts b/src/errors.ts index 24dc276c..5dd50b39 100644 --- a/src/errors.ts +++ b/src/errors.ts @@ -1,5 +1,66 @@ +import { WebServiceClientError } from './types.js'; + /* tslint:disable:max-classes-per-file */ +/** + * An error returned by the GeoIP2 web service or encountered while + * communicating with it. + * + * In addition to the standard `Error` properties (including `cause`, which + * holds the underlying error when one exists, such as the network error + * behind a `FETCH_ERROR`), it exposes the `code`, `status`, and `url` + * associated with the failure. + */ +export class WebServiceError extends Error implements WebServiceClientError { + /** + * The error code returned by the web service or generated by this client. + */ + public readonly code: string; + /** + * A human-readable description of the error. This is an alias of `message`, + * retained for backward compatibility. + */ + public readonly error: string; + /** + * The HTTP status code, when the error originated from an HTTP response. + * + * Declared with `declare` so that no class field is emitted: the property is + * absent (rather than set to `undefined`) when no status applies, e.g. on + * network-level errors such as `FETCH_ERROR` and `NETWORK_TIMEOUT`. + */ + declare public readonly status?: number; + /** + * The URL that was being requested when the error occurred. + */ + public readonly url: string; + + constructor( + properties: { + code: string; + error: string; + status?: number; + url: string; + }, + options?: { cause?: unknown } + ) { + super(properties.error, options); + this.code = properties.code; + this.error = properties.error; + // Only assign `status` when present so it stays genuinely optional: on + // network-level errors (e.g. FETCH_ERROR, NETWORK_TIMEOUT) the instance + // carries no `status` property at all, rather than `status: undefined`. + if (properties.status !== undefined) { + this.status = properties.status; + } + this.url = properties.url; + } +} + +// Set `name` on the prototype rather than in the constructor. Using a string +// literal keeps the name correct under minification, and keeping it off the +// instance means it stays out of `JSON.stringify` output. +WebServiceError.prototype.name = 'WebServiceError'; + /** * This error is thrown when the IP address is not found in the database. * This generally means that the address was a private or reserved address. diff --git a/src/index.ts b/src/index.ts index 230561d2..174d1196 100644 --- a/src/index.ts +++ b/src/index.ts @@ -6,3 +6,4 @@ export { Reader, ReaderModel, WebServiceClient }; export * from './records.js'; export * from './models/index.js'; export * from './errors.js'; +export type { WebServiceClientError } from './types.js'; diff --git a/src/types.ts b/src/types.ts index f1df0263..e28740cc 100644 --- a/src/types.ts +++ b/src/types.ts @@ -14,6 +14,11 @@ export interface WebServiceClientError { error: string; status?: number; url: string; + /** + * The underlying error that caused this one, when available (for example, + * the network error behind a `FETCH_ERROR`). + */ + cause?: unknown; } export interface Json { diff --git a/src/webServiceClient.spec.ts b/src/webServiceClient.spec.ts index c5591e09..9d0818ba 100644 --- a/src/webServiceClient.spec.ts +++ b/src/webServiceClient.spec.ts @@ -1,5 +1,6 @@ import nock from 'nock'; import geoip2Fixture from '../fixtures/geoip2.json' with { type: 'json' }; +import { WebServiceError } from './errors.js'; import Client from './webServiceClient.js'; import * as models from './models/index.js'; @@ -12,6 +13,41 @@ const auth = { user: '123', }; +const expectError = async ( + promise: Promise, + expected: { + code: string; + error: string; + status?: number; + url: string; + // Whether the error's `cause` should be present or absent. + cause?: 'defined' | 'undefined'; + } +): Promise => { + const { cause, ...fields } = expected; + const err = await promise.then( + () => { + throw new Error('expected the request to reject'); + }, + (e: unknown) => e + ); + expect(err).toBeInstanceOf(WebServiceError); + const webServiceError = err as WebServiceError; + expect(webServiceError).toMatchObject(fields); + // The `error` property is retained as an alias of `message`. + expect(webServiceError.message).toBe(expected.error); + // Assert `status` explicitly (toMatchObject ignores extra own properties), + // so a regression leaking a `status` onto a network-error path is caught. + expect(webServiceError.status).toBe(expected.status); + if (cause === 'defined') { + expect(webServiceError.cause).toBeDefined(); + } + if (cause === 'undefined') { + expect(webServiceError.cause).toBeUndefined(); + } + return webServiceError; +}; + describe('WebServiceClient', () => { afterEach(() => { nock.cleanAll(); @@ -410,102 +446,151 @@ describe('WebServiceClient', () => { .delay(200) // Delay the response to trigger the timeout .reply(200, geoip2Fixture); - await expect(client.city(ip)).rejects.toEqual({ + // The underlying abort/timeout error is preserved as the cause. + await expectError(client.city(ip), { code: 'NETWORK_TIMEOUT', error: 'The request timed out', url: `https://geoip.maxmind.com/geoip/v2.1/city/${ip}`, + cause: 'defined', }); }); }); describe('error handling', () => { - it('rejects if the IP address is invalid', () => { + it('rejects if the IP address is invalid', async () => { const ip = 'foo'; - expect.assertions(1); - return expect(client.city(ip)).rejects.toEqual({ + await expectError(client.city(ip), { code: 'IP_ADDRESS_INVALID', error: 'The IP address provided is invalid', url: baseUrl + fullPath('city', ip), + cause: 'undefined', }); }); - it('handles 5xx level errors', () => { + it('handles 5xx level errors', async () => { const ip = '8.8.8.8'; - expect.assertions(1); nockInstance.get(fullPath('city', ip)).basicAuth(auth).reply(500); - return expect(client.city(ip)).rejects.toEqual({ + await expectError(client.city(ip), { code: 'SERVER_ERROR', error: 'Received a server error with HTTP status code: 500', status: 500, url: baseUrl + fullPath('city', ip), + cause: 'undefined', }); }); - it('handles 3xx level errors', () => { + it('handles 3xx level errors', async () => { const ip = '8.8.8.8'; - expect.assertions(1); nockInstance.get(fullPath('city', ip)).basicAuth(auth).reply(300); - return expect(client.city(ip)).rejects.toEqual({ + await expectError(client.city(ip), { code: 'HTTP_STATUS_CODE_ERROR', error: 'Received an unexpected HTTP status code: 300', status: 300, url: baseUrl + fullPath('city', ip), + cause: 'undefined', }); }); - it('handles errors with unknown payload', () => { + it('handles errors with unknown payload', async () => { const ip = '8.8.8.8'; - expect.assertions(1); nockInstance .get(fullPath('city', ip)) .basicAuth(auth) .reply(401, { foo: 'bar' }); - return expect(client.city(ip)).rejects.toEqual({ + await expectError(client.city(ip), { code: 'INVALID_RESPONSE_BODY', error: 'Received an invalid or unparseable response body', status: 401, url: baseUrl + fullPath('city', ip), + cause: 'undefined', }); }); - it('handles 200s with bad json', () => { + test.each` + description | payload + ${'a non-string code'} | ${{ code: 123, error: 'an error' }} + ${'a non-string error'} | ${{ code: 'A_CODE', error: {} }} + ${'empty string fields'} | ${{ code: '', error: '' }} + `( + 'treats $description as an invalid response body', + async ({ payload }) => { + const ip = '8.8.8.8'; + + nockInstance + .get(fullPath('city', ip)) + .basicAuth(auth) + .reply(400, payload); + + await expectError(client.city(ip), { + code: 'INVALID_RESPONSE_BODY', + error: 'Received an invalid or unparseable response body', + status: 400, + url: baseUrl + fullPath('city', ip), + cause: 'undefined', + }); + } + ); + + it('handles 200s with bad json', async () => { const ip = '8.8.8.8'; - expect.assertions(1); nockInstance.get(fullPath('city', ip)).basicAuth(auth).reply(200, 'foo'); - return expect(client.city(ip)).rejects.toEqual({ + const err = await expectError(client.city(ip), { code: 'INVALID_RESPONSE_BODY', error: 'Received an invalid or unparseable response body', url: baseUrl + fullPath('city', ip), + cause: 'defined', }); + // The JSON parse error is preserved as the cause. (instanceof Error is + // unreliable here: under --experimental-vm-modules the parse error is + // created in a different realm than this test file.) + expect((err.cause as Error).message).toEqual(expect.any(String)); }); - it('handles general network errors', () => { + it('preserves the cause when an error response body is not JSON', async () => { const ip = '8.8.8.8'; - const error = 'Network Error'; - const expected = { - code: 'FETCH_ERROR', - error: `Error - ${error}`, + nockInstance + .get(fullPath('city', ip)) + .basicAuth(auth) + .reply(401, 'this is not json'); + + const err = await expectError(client.city(ip), { + code: 'INVALID_RESPONSE_BODY', + error: 'Received an invalid or unparseable response body', + status: 401, url: baseUrl + fullPath('city', ip), - }; + cause: 'defined', + }); + // The parse failure on a non-2xx response is preserved as the cause. + expect((err.cause as Error).message).toEqual(expect.any(String)); + }); - expect.assertions(1); + it('handles general network errors', async () => { + const ip = '8.8.8.8'; + const error = 'Network Error'; nockInstance .get(fullPath('city', ip)) .basicAuth(auth) .replyWithError(error); - return expect(client.city(ip)).rejects.toEqual(expected); + const err = await expectError(client.city(ip), { + code: 'FETCH_ERROR', + error: `Error - ${error}`, + url: baseUrl + fullPath('city', ip), + cause: 'defined', + }); + // The original fetch error is preserved as the cause. + expect((err.cause as Error).message).toBe(error); }); test.each` @@ -519,19 +604,19 @@ describe('WebServiceClient', () => { ${401} | ${'USER_ID_REQUIRED'} | ${'user id required'} ${402} | ${'OUT_OF_QUERIES'} | ${'out of queries'} ${403} | ${'PERMISSION_REQUIRED'} | ${'permission required'} - `('handles $code error', ({ code, error, status }) => { + `('handles $code error', async ({ code, error, status }) => { const ip = '8.8.8.8'; nockInstance .get(fullPath('city', ip)) .basicAuth(auth) .reply(status, { code, error }); - expect.assertions(1); - return expect(client.city(ip)).rejects.toEqual({ + await expectError(client.city(ip), { code, error, status, + cause: 'undefined', url: baseUrl + fullPath('city', ip), }); }); diff --git a/src/webServiceClient.ts b/src/webServiceClient.ts index af8633bf..9d24e2fc 100644 --- a/src/webServiceClient.ts +++ b/src/webServiceClient.ts @@ -1,7 +1,7 @@ import * as mmdb from 'maxmind'; import packageInfo from '../package.json' with { type: 'json' }; +import { WebServiceError } from './errors.js'; import * as models from './models/index.js'; -import { WebServiceClientError } from './types.js'; /** Option for the WebServiceClient constructor */ interface Options { @@ -16,10 +16,6 @@ interface Options { /** The timeout. The default is 3000 */ timeout?: number; } -interface ResponseError { - code?: string; - error?: string; -} type servicePath = 'city' | 'country' | 'insights'; const invalidResponseBody = { @@ -27,6 +23,14 @@ const invalidResponseBody = { error: 'Received an invalid or unparseable response body', }; +const isErrorBody = (data: unknown): data is { code: string; error: string } => + typeof data === 'object' && + data !== null && + typeof (data as Record).code === 'string' && + (data as Record).code !== '' && + typeof (data as Record).error === 'string' && + (data as Record).error !== ''; + /** Class representing the WebServiceClient **/ export default class WebServiceClient { private accountID: string; @@ -121,11 +125,11 @@ export default class WebServiceClient { const url = `https://${this.host}${parsedPath}`; if (!mmdb.validate(ipAddress)) { - throw { + throw new WebServiceError({ code: 'IP_ADDRESS_INVALID', error: 'The IP address provided is invalid', url, - }; + }); } const options: RequestInit = { @@ -147,17 +151,23 @@ export default class WebServiceClient { ? err : new Error(String(err)); if (error.name === 'TimeoutError') { - throw { - code: 'NETWORK_TIMEOUT', - error: 'The request timed out', - url, - }; + throw new WebServiceError( + { + code: 'NETWORK_TIMEOUT', + error: 'The request timed out', + url, + }, + { cause: error } + ); } - throw { - code: 'FETCH_ERROR', - error: `${error.name} - ${error.message}`, - url, - }; + throw new WebServiceError( + { + code: 'FETCH_ERROR', + error: `${error.name} - ${error.message}`, + url, + }, + { cause: error } + ); } if (!response.ok) { @@ -167,8 +177,11 @@ export default class WebServiceClient { let data; try { data = await response.json(); - } catch { - throw { ...invalidResponseBody, url }; + } catch (err) { + throw new WebServiceError( + { ...invalidResponseBody, url }, + { cause: err } + ); } return new modelClass(data); @@ -177,38 +190,46 @@ export default class WebServiceClient { private async handleError( response: Response, url: string - ): Promise { + ): Promise { const status = response.status; if (status && status >= 500 && status < 600) { - return { + return new WebServiceError({ code: 'SERVER_ERROR', error: `Received a server error with HTTP status code: ${status}`, status, url, - }; + }); } if (status && (status < 400 || status >= 600)) { - return { + return new WebServiceError({ code: 'HTTP_STATUS_CODE_ERROR', error: `Received an unexpected HTTP status code: ${status}`, status, url, - }; + }); } - let data; + let data: unknown; try { - data = (await response.json()) as ResponseError; + data = await response.json(); + } catch (err) { + return new WebServiceError( + { ...invalidResponseBody, status, url }, + { cause: err } + ); + } - if (!data.code || !data.error) { - return { ...invalidResponseBody, status, url }; - } - } catch { - return { ...invalidResponseBody, status, url }; + if (!isErrorBody(data)) { + return new WebServiceError({ ...invalidResponseBody, status, url }); } - return { ...data, status, url } as WebServiceClientError; + return new WebServiceError({ + code: data.code, + error: data.error, + status, + url, + }); } } From 6d49d03d71a43b181ff6cead62f8bb382026a381 Mon Sep 17 00:00:00 2001 From: Gregory Oschwald Date: Wed, 24 Jun 2026 16:57:34 +0000 Subject: [PATCH 2/2] Include the underlying cause in the FETCH_ERROR message A FETCH_ERROR previously surfaced only "TypeError - fetch failed", with the real reason (DNS failure, refused connection, TLS error, etc.) hidden in the cause. Consumers that log only `code`/`error` never saw it. Append the underlying cause's message to the error string so the reason is visible without inspecting `cause`, which is still attached. STF-803 Co-Authored-By: Claude Opus 4.8 (1M context) --- README.md | 4 +++- src/webServiceClient.spec.ts | 21 +++++++++++++++++++++ src/webServiceClient.ts | 7 ++++++- 3 files changed, 30 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3342a41d..7578821c 100644 --- a/README.md +++ b/README.md @@ -132,7 +132,9 @@ codes are provided: * `HTTP_STATUS_CODE_ERROR` for unexpected HTTP status codes * `INVALID_RESPONSE_BODY` for invalid JSON responses or unparseable response bodies * `NETWORK_TIMEOUT` for network request timeouts -* `FETCH_ERROR` for internal `fetch` errors +* `FETCH_ERROR` for internal `fetch` errors. The `error` message includes the + underlying failure reason (e.g. a DNS or connection error) when available, + and the original error is attached as `cause`. ## Database Usage diff --git a/src/webServiceClient.spec.ts b/src/webServiceClient.spec.ts index 9d0818ba..c7d4369c 100644 --- a/src/webServiceClient.spec.ts +++ b/src/webServiceClient.spec.ts @@ -593,6 +593,27 @@ describe('WebServiceClient', () => { expect((err.cause as Error).message).toBe(error); }); + it('includes the underlying cause in the FETCH_ERROR message', async () => { + const ip = '8.8.8.8'; + const fetchError = Object.assign(new TypeError('fetch failed'), { + cause: new Error('connect ECONNREFUSED 1.2.3.4:443'), + }); + + nockInstance + .get(fullPath('city', ip)) + .basicAuth(auth) + .replyWithError(fetchError); + + const err = await expectError(client.city(ip), { + code: 'FETCH_ERROR', + error: 'TypeError - fetch failed: connect ECONNREFUSED 1.2.3.4:443', + url: baseUrl + fullPath('city', ip), + cause: 'defined', + }); + // The original error (with its own cause) is still attached. + expect((err.cause as Error).message).toBe('fetch failed'); + }); + test.each` status | code | error ${400} | ${'IP_ADDRESS_INVALID'} | ${'ip address invalid'} diff --git a/src/webServiceClient.ts b/src/webServiceClient.ts index 9d24e2fc..139f3e5f 100644 --- a/src/webServiceClient.ts +++ b/src/webServiceClient.ts @@ -160,10 +160,15 @@ export default class WebServiceClient { { cause: error } ); } + // Include the underlying cause's message in the error string so the + // reason (e.g. a DNS or connection failure) is visible to consumers that + // only log `code`/`error`, not just available via `cause`. + const causeDetail = + error.cause instanceof Error ? `: ${error.cause.message}` : ''; throw new WebServiceError( { code: 'FETCH_ERROR', - error: `${error.name} - ${error.message}`, + error: `${error.name} - ${error.message}${causeDetail}`, url, }, { cause: error }