From 8914dab6ffc3e9d85783bb8d0c4d3072f1045911 Mon Sep 17 00:00:00 2001 From: Alan Plum Date: Wed, 27 Nov 2024 09:57:26 +0100 Subject: [PATCH] Revert "Change request/error handling" This reverts commit f22e90f43e7f3c7d0ce1a102da0ee3f0e5bdc7f5. This reverts commit defd6aae86fe3ff0b4437c226870703a6709738e. --- CHANGELOG.md | 114 ----------- README.md | 52 ++--- src/connection.ts | 226 ++++++++++------------ src/database.ts | 28 ++- src/error.ts | 323 +++++++------------------------- src/job.ts | 8 +- src/lib/request.ts | 96 +++++----- src/route.ts | 39 ++-- src/test/27-query-management.ts | 4 +- 9 files changed, 263 insertions(+), 627 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index c4c9d3cd..e92a2a25 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,59 +16,6 @@ This driver uses semantic versioning: ## [Unreleased] -### Changed - -- Errors encountered before a request completes are now wrapped in a - `NetworkError` or a subclass thereof - - This should help making it easier to diagnose network issues and distinguish - the relevant error conditions. - - The originating error can still be accessed using the `cause` property of the - `NetworkError` error. - -- `HttpError` now extends the `NetworkError` class - - This allows treating all non-`ArangoError` errors as one category of errors, - even when there is no server response available. - -- `db.waitForPropagation` now throws a `PropagationTimeoutError` error when - invoked with a `timeout` option and the timeout duration is exceeded - - The method would previously throw the most recent error encountered while - waiting for replication. The originating error can still be accessed using - the `cause` property of the `PropagationTimeoutError` error. - -- `db.waitForPropagation` now respects the `timeout` option more strictly - - Previously the method would only time out if the timeout duration was - exceeded after the most recent request failed. Now the timeout is - recalculated and passed on to each request, preventing it from exceeding - the specified duration. - - If the propagation timed out due to an underlying request exceeding the - timeout duration, the `cause` property of the `PropagationTimeoutError` - error will be a `ResponseTimeoutError` error. - -- `config.beforeRequest` and `config.afterResponse` callbacks can now return - promises - - If the callback returns a promise, it will be awaited before the request - and response cycle proceeds. If either callback throws an error or returns - a promise that is rejected, that error will be thrown instead. - -- `config.afterResponse` callback signature changed - - The callback signature previously used the internal `ArangojsResponse` type. - The new signature uses the `Response` type of the Fetch API with an - additional `request` property to more accurately represent the actual value - it receives as the `parsedBody` property will never be present. - -- `response` property on `ArangoError` is now optional - - This property should always be present but this allows using the error in - situations where a response might not be available. - ### Added - Added `database.availability` method @@ -79,67 +26,6 @@ This driver uses semantic versioning: - Added `database.supportInfo` method -- Added `onError` option to `Config` (DE-955) - - This option can be used to specify a callback function that will be invoked - whenever a request results in an error. Unlike `afterResponse`, this callback - will be invoked even if the request completed but returned an error status. - In this case the error will be the `HttpError` or `ArangoError` representing - the error response. - - If the `onError` callback throws an error or returns a promise that is - rejected, that error will be thrown instead. - -- Added `NetworkError` class - - This is the common base class for all errors (including `HttpError`) that - occur while making a request. The originating error can be accessed using the - `cause` property. The request object can be accessed using the `request` - property. - - Note that `ArangoError` and the new `PropagationTimeoutError` error type - do not extend `NetworkError` but may wrap an underlying error, which can - be accessed using the `cause` property. - -- Added `ResponseTimeoutError` class - - This error extends `NetworkError` and is thrown when a request deliberately - times out using the `timeout` option. - -- Added `RequestAbortedError` class - - This error extends `NetworkError` and is thrown when a request is aborted - by using the `db.close` method. - -- Added `FetchFailedError` class - - This error extends `NetworkError` and is thrown when a request fails because - the underlying `fetch` call fails (usually with a `TypeError`). - - In Node.js the root cause of this error (e.g. a network failure) can often be - found in the `cause` property of the originating error, i.e. the `cause` - property of the `cause` property of this error. - - In browsers the root cause is usually not exposed directly but can often - be diagnosed by examining the developer console or network tab. - -- Added `PropagationTimeoutError` class - - This error does not extend `NetworkError` but wraps the most recent error - encountered while waiting for replication, which can be accessed using the - `cause` property. This error is only thrown when `db.waitForPropagation` - is invoked with a `timeout` option and the timeout duration is exceeded. - -- Added `ProcessedResponse` type - - This type replaces the previously internal `ArangojsResponse` type and - extends the native `Response` type with additional properties. - -- Added optional `ArangoError#request` property - - This property is always present if the error has a `response` property. In - normal use this should always be the case. - - Added `keepNull` option to `CollectionInsertOptions` type (DE-946) This option was previously missing from the type. diff --git a/README.md b/README.md index 5c48a172..8f81115a 100644 --- a/README.md +++ b/README.md @@ -139,42 +139,21 @@ and [the `db` object](https://www.arangodb.com/docs/stable/appendix-references-d ## Error responses -If the server returns an ArangoDB error response, arangojs will throw an -`ArangoError` with an `errorNum` property indicating the ArangoDB error code -and expose the response body as the `response` property of the error object. +If arangojs encounters an API error, it will throw an `ArangoError` with +an `errorNum` property indicating the ArangoDB error code and the `code` +property indicating the HTTP status code from the response body. -For all other errors during the request/response cycle, arangojs will throw a -`NetworkError` or a more specific subclass thereof and expose the originating -request object as the `request` property of the error object. +For any other non-ArangoDB error responses (4xx/5xx status code), it will throw +an `HttpError` error with the status code indicated by the `code` property. -If the server responded with a non-2xx status code, this `NetworkError` will -be an `HttpError` with a `code` property indicating the HTTP status code of the -response and a `response` property containing the response object itself. +If the server response did not indicate an error but the response body could +not be parsed, a regular `SyntaxError` may be thrown instead. -If the error is caused by an exception, the originating exception will be -available as the `cause` property of the error object thrown by arangojs. For -network errors, this will often be a `TypeError`. +In all of these cases the server response object will be exposed as the +`response` property on the error object. -### Node.js network errors - -In Node.js, network errors caused by a `TypeError` will often have a `cause` -property containing a more detailed exception. - -Specifically, these are often either system errors (represented by regular -`Error` objects with additional properties) or errors from the `undici` module -Node.js uses internally for its native `fetch` implementation. - -Node.js system error objects provide a `code` property containing the specific -string error code, a `syscall` property identifying the underlying system call -that triggered the error (e.g. `connect`), as well as other helpful properties. - -For more details on Node.js system errors, see the Node.js documentation of the -[`SystemError` interface](https://nodejs.org/api/errors.html#class-systemerror) -as well as the section on -[Node.js error codes](https://nodejs.org/api/errors.html#nodejs-error-codes). - -For more details on the errors thrown by `undici`, see the -[undici errors documentation](https://undici.nodejs.org/#/docs/api/Errors.md). +If the request failed at a network level or the connection was closed without +receiving a response, the underlying system error will be thrown instead. ## Common issues @@ -191,15 +170,6 @@ Additionally please ensure that your version of Node.js (or browser) and ArangoDB are supported by the version of arangojs you are trying to use. See the [compatibility section](#compatibility) for additional information. -You can install an older version of arangojs using `npm` or `yarn`: - -```sh -# for version 8.x.x -yarn add arangojs@8 -# - or - -npm install --save arangojs@8 -``` - ### No code intelligence when using require instead of import If you are using `require` to import the `arangojs` module in JavaScript, the diff --git a/src/connection.ts b/src/connection.ts index 645830ac..2565226e 100644 --- a/src/connection.ts +++ b/src/connection.ts @@ -13,17 +13,18 @@ import { Database } from "./database.js"; import { ArangoError, HttpError, - NetworkError, - PropagationTimeoutError, isArangoError, isArangoErrorResponse, - isNetworkError, + isSystemError, } from "./error.js"; import { ERROR_ARANGO_CONFLICT, + ERROR_ARANGO_MAINTENANCE_MODE, } from "./lib/codes.js"; import { normalizeUrl } from "./lib/normalizeUrl.js"; import { + ArangojsError, + ArangojsResponse, createRequest, RequestConfig, RequestFunction, @@ -161,6 +162,8 @@ export type RequestOptions = { /** * Time in milliseconds after which arangojs will abort the request if the * socket has not already timed out. + * + * See also `agentOptions.timeout` in {@link Config}. */ timeout?: number; /** @@ -177,37 +180,17 @@ export type RequestOptions = { search?: URLSearchParams | Record; }; -/** - * Processed response object. - */ -export interface ProcessedResponse extends globalThis.Response { - /** - * @internal - * - * Identifier of the ArangoDB host that served this request. - */ - arangojsHostUrl?: string; - /** - * Fetch request object. - */ - request: globalThis.Request; - /** - * Parsed response body. - */ - parsedBody?: T; -}; - /** * @internal */ -type Task = { +type Task = { hostUrl?: string; stack?: () => string; allowDirtyRead: boolean; retryOnConflict: number; - resolve: (result: T) => void; - reject: (error: unknown) => void; - transform?: (res: ProcessedResponse) => T; + resolve: (result: any) => void; + reject: (error: Error) => void; + transform?: (res: ArangojsResponse) => any; retries: number; options: { method: string; @@ -363,7 +346,7 @@ export type Config = { * * @param req - Request object or XHR instance used for this request. */ - beforeRequest?: (req: globalThis.Request) => void | Promise; + beforeRequest?: (req: globalThis.Request) => void; /** * Callback that will be invoked when the server response has been received * and processed or when the request has been failed without a response. @@ -374,13 +357,7 @@ export type Config = { * @param err - Error encountered when handling this request or `null`. * @param res - Response object for this request, if no error occurred. */ - afterResponse?: (err: NetworkError | null, res?: globalThis.Response & { request: globalThis.Request }) => void | Promise; - /** - * Callback that will be invoked when a request - * - * @param err - Error encountered when handling this request. - */ - onError?: (err: Error) => void | Promise; + afterResponse?: (err: ArangojsError | null, res?: ArangojsResponse) => void; /** * If set to a positive number, requests will automatically be retried at * most this many times if they result in a write-write conflict. @@ -448,7 +425,6 @@ export class Connection { protected _activeHostUrl: string; protected _activeDirtyHostUrl: string; protected _transactionId: string | null = null; - protected _onError?: (err: Error) => void | Promise; protected _precaptureStackTraces: boolean; protected _queueTimes = new LinkedList<[number, number]>(); protected _responseQueueTimeSamples: number; @@ -490,7 +466,6 @@ export class Connection { this._precaptureStackTraces = Boolean(config.precaptureStackTraces); this._responseQueueTimeSamples = config.responseQueueTimeSamples ?? 10; this._retryOnConflict = config.retryOnConflict ?? 0; - this._onError = config.onError; if (this._responseQueueTimeSamples < 0) { this._responseQueueTimeSamples = Infinity; } @@ -545,29 +520,31 @@ export class Connection { } protected async _runQueue() { - if (this._activeTasks >= this._taskPoolSize) return; - const task = this._queue.shift(); - if (!task) return; + if (!this._queue.length || this._activeTasks >= this._taskPoolSize) return; + const task = this._queue.shift()!; let hostUrl = this._activeHostUrl; + if (task.hostUrl !== undefined) { + hostUrl = task.hostUrl; + } else if (task.allowDirtyRead) { + hostUrl = this._activeDirtyHostUrl; + this._activeDirtyHostUrl = + this._hostUrls[ + (this._hostUrls.indexOf(this._activeDirtyHostUrl) + 1) % + this._hostUrls.length + ]; + task.options.headers.set("x-arango-allow-dirty-read", "true"); + } else if (this._loadBalancingStrategy === "ROUND_ROBIN") { + this._activeHostUrl = + this._hostUrls[ + (this._hostUrls.indexOf(this._activeHostUrl) + 1) % + this._hostUrls.length + ]; + } + this._activeTasks += 1; try { - this._activeTasks += 1; - if (task.hostUrl !== undefined) { - hostUrl = task.hostUrl; - } else if (task.allowDirtyRead) { - hostUrl = this._activeDirtyHostUrl; - const i = this._hostUrls.indexOf(this._activeDirtyHostUrl) + 1; - this._activeDirtyHostUrl = this._hostUrls[i % this._hostUrls.length]; - } else if (this._loadBalancingStrategy === "ROUND_ROBIN") { - const i = this._hostUrls.indexOf(this._activeHostUrl) + 1; - this._activeHostUrl = this._hostUrls[i % this._hostUrls.length]; - } - const res: globalThis.Response & { - request: globalThis.Request; - arangojsHostUrl: string; - parsedBody?: any; - } = Object.assign(await this._hosts[this._hostUrls.indexOf(hostUrl)]( + const res = await this._hosts[this._hostUrls.indexOf(hostUrl)]( task.options - ), { arangojsHostUrl: hostUrl }); + ); const leaderEndpoint = res.headers.get(LEADER_ENDPOINT_HEADER); if (res.status === 503 && leaderEndpoint) { const [cleanUrl] = this.addToHostList(leaderEndpoint); @@ -576,54 +553,62 @@ export class Connection { this._activeHostUrl = cleanUrl; } this._queue.push(task); - return; - } - const queueTime = res.headers.get("x-arango-queue-time-seconds"); - if (queueTime) { - this._queueTimes.push([Date.now(), Number(queueTime)]); - while (this._responseQueueTimeSamples < this._queueTimes.length) { - this._queueTimes.shift(); + } else { + res.arangojsHostUrl = hostUrl; + const contentType = res.headers.get("content-type"); + const queueTime = res.headers.get("x-arango-queue-time-seconds"); + if (queueTime) { + this._queueTimes.push([Date.now(), Number(queueTime)]); + while (this._responseQueueTimeSamples < this._queueTimes.length) { + this._queueTimes.shift(); + } } - } - const contentType = res.headers.get("content-type"); - if (res.status >= 400) { - if (contentType?.match(MIME_JSON)) { - const errorResponse = res.clone(); - let errorBody: any; + if (res.status >= 400) { try { - errorBody = await errorResponse.json(); - } catch { - // noop - } - if (isArangoErrorResponse(errorBody)) { - res.parsedBody = errorBody; - throw ArangoError.from(res); + if (contentType?.match(MIME_JSON)) { + const errorResponse = res.clone(); + let errorBody: any; + try { + errorBody = await errorResponse.json(); + } catch { + // noop + } + if (isArangoErrorResponse(errorBody)) { + res.parsedBody = errorBody; + throw new ArangoError(res); + } + } + throw new HttpError(res); + } catch (err: any) { + if (task.stack) { + err.stack += task.stack(); + } + throw err; } } - throw new HttpError(res); - } - if (res.body) { - if (task.options.expectBinary) { - res.parsedBody = await res.blob(); - } else if (contentType?.match(MIME_JSON)) { - res.parsedBody = await res.json(); - } else { - res.parsedBody = await res.text(); + if (res.body) { + if (task.options.expectBinary) { + res.parsedBody = await res.blob(); + } else if (contentType?.match(MIME_JSON)) { + res.parsedBody = await res.json(); + } else { + res.parsedBody = await res.text(); + } } + task.resolve(task.transform ? task.transform(res) : res); } - let result: any = res; - if (task.transform) result = task.transform(res); - task.resolve(result); - } catch (e: unknown) { - const err = e as Error; + } catch (err: any) { if ( !task.allowDirtyRead && this._hosts.length > 1 && this._activeHostUrl === hostUrl && this._loadBalancingStrategy !== "ROUND_ROBIN" ) { - const i = this._hostUrls.indexOf(this._activeHostUrl) + 1; - this._activeHostUrl = this._hostUrls[i % this._hostUrls.length]; + this._activeHostUrl = + this._hostUrls[ + (this._hostUrls.indexOf(this._activeHostUrl) + 1) % + this._hostUrls.length + ]; } if ( isArangoError(err) && @@ -632,37 +617,28 @@ export class Connection { ) { task.retryOnConflict -= 1; this._queue.push(task); - return; - } - if ( - (isNetworkError(err) || isArangoError(err)) && - err.isSafeToRetry && + } else if ( + ((isSystemError(err) && + err.syscall === "connect" && + err.code === "ECONNREFUSED") || + (isArangoError(err) && + err.errorNum === ERROR_ARANGO_MAINTENANCE_MODE)) && task.hostUrl === undefined && this._maxRetries !== false && task.retries < (this._maxRetries || this._hosts.length - 1) ) { task.retries += 1; this._queue.push(task); - return; - } - if (task.stack) { - err.stack += task.stack(); - } - if (this._onError) { - try { - const p = this._onError(err); - if (p instanceof Promise) await p; - } catch (e) { - (e as Error).cause = err; - task.reject(e); - return; + } else { + if (task.stack) { + err.stack += task.stack(); } + task.reject(err); } - task.reject(err); } finally { this._activeTasks -= 1; - setTimeout(() => this._runQueue(), 0); } + this._runQueue(); } setBearerAuth(auth: BearerAuthCredentials) { @@ -856,7 +832,6 @@ export class Connection { const numHosts = this._hosts.length; const propagated = [] as string[]; const started = Date.now(); - const endOfTime = started + timeout; let index = 0; while (true) { if (propagated.length === numHosts) { @@ -867,17 +842,10 @@ export class Connection { } const hostUrl = this._hostUrls[index]; try { - await this.request({ - ...request, - hostUrl, - timeout: endOfTime - Date.now(), - }); - } catch (e) { - if (endOfTime < Date.now()) { - throw new PropagationTimeoutError( - undefined, - { cause: e as Error } - ); + await this.request({ ...request, hostUrl }); + } catch (e: any) { + if (started + timeout < Date.now()) { + throw e; } await new Promise((resolve) => setTimeout(resolve, 1000)); continue; @@ -893,7 +861,7 @@ export class Connection { * * Performs a request using the arangojs connection pool. */ - request( + request( { hostUrl, method = "GET", @@ -908,7 +876,7 @@ export class Connection { path, search: params, }: RequestOptions, - transform?: (res: globalThis.Response & { request: globalThis.Request; parsedBody?: any }) => T + transform?: (res: ArangojsResponse) => T ): Promise { return new Promise((resolve, reject) => { const headers = mergeHeaders(this._headers, requestHeaders ?? {}); @@ -933,10 +901,6 @@ export class Connection { headers.set("x-arango-trx-id", this._transactionId); } - if (allowDirtyRead) { - headers.set("x-arango-allow-dirty-read", "true"); - } - const task: Task = { retries: 0, hostUrl, diff --git a/src/database.ts b/src/database.ts index d53f4862..13836284 100644 --- a/src/database.ts +++ b/src/database.ts @@ -29,7 +29,6 @@ import { } from "./collection.js"; import { ArangoApiResponse, - ProcessedResponse, Config, Connection, RequestOptions, @@ -45,6 +44,7 @@ import { } from "./graph.js"; import { Job } from "./job.js"; import { DATABASE_NOT_FOUND } from "./lib/codes.js"; +import { ArangojsResponse } from "./lib/request.js"; import { Route } from "./route.js"; import { Transaction } from "./transaction.js"; import { CreateViewOptions, View, ViewDescription } from "./view.js"; @@ -2064,10 +2064,10 @@ type TrappedError = { /** * @internal */ -type TrappedRequest = { +type TrappedRequest = { error?: false; jobId: string; - onResolve: (res: ProcessedResponse) => void; + onResolve: (res: ArangojsResponse) => void; onReject: (error: any) => void; }; @@ -2082,9 +2082,7 @@ export class Database { protected _collections = new Map(); protected _graphs = new Map(); protected _views = new Map(); - protected _trapRequest?: ( - trapped: TrappedError | TrappedRequest - ) => void; + protected _trapRequest?: (trapped: TrappedError | TrappedRequest) => void; /** * Creates a new `Database` instance with its own connection pool. @@ -2195,14 +2193,14 @@ export class Database { * If `absolutePath` is set to `true`, the database path will not be * automatically prepended to the `basePath`. * - * @param T - Return type to use. Defaults to the response object type. + * @param ReturnType - Return type to use. Defaults to the response object type. * @param options - Options for this request. * @param transform - An optional function to transform the low-level * response object to a more useful return value. */ - async request( + async request( options: RequestOptions & { absolutePath?: boolean }, - transform?: (res: ProcessedResponse) => ReturnType + transform?: (res: ArangojsResponse) => ReturnType ): Promise; /** * @internal @@ -2216,17 +2214,17 @@ export class Database { * @param transform - If set to `false`, the raw response object will be * returned. */ - async request( + async request( options: RequestOptions & { absolutePath?: boolean }, transform: false - ): Promise>; - async request( + ): Promise; + async request( { absolutePath = false, basePath, ...opts }: RequestOptions & { absolutePath?: boolean }, - transform: false | ((res: ProcessedResponse) => ReturnType) = (res) => res.parsedBody as ReturnType + transform: false | ((res: ArangojsResponse) => ReturnType) = (res) => res.parsedBody ): Promise { if (!absolutePath) { basePath = `/_db/${encodeURIComponent(this._name)}${basePath || ""}`; @@ -2238,7 +2236,7 @@ export class Database { const options = { ...opts }; options.headers = new Headers(options.headers); options.headers.set("x-arango-async", "store"); - let jobRes: ProcessedResponse; + let jobRes: ArangojsResponse; try { jobRes = await this._connection.request({ basePath, ...options }); } catch (e) { @@ -6412,7 +6410,7 @@ export class Database { * ``` */ async createJob(callback: () => Promise): Promise> { - const trap = new Promise>((resolveTrap) => { + const trap = new Promise((resolveTrap) => { this._trapRequest = (trapped) => resolveTrap(trapped); }); const eventualResult = callback(); diff --git a/src/error.ts b/src/error.ts index 84bcd72b..02e5f539 100644 --- a/src/error.ts +++ b/src/error.ts @@ -9,8 +9,7 @@ * @packageDocumentation */ -import { ProcessedResponse } from "./connection.js"; -import { ERROR_ARANGO_MAINTENANCE_MODE } from "./lib/codes.js"; +import { ArangojsResponse } from "./lib/request.js"; const messages: { [key: number]: string } = { 0: "Network Error", @@ -59,6 +58,15 @@ const messages: { [key: number]: string } = { 599: "Network Connect Timeout Error", }; +const nativeErrorKeys = [ + "fileName", + "lineNumber", + "columnNumber", + "stack", + "description", + "number", +] as (keyof Error)[]; + /** * Indicates whether the given value represents an {@link ArangoError}. * @@ -69,326 +77,129 @@ export function isArangoError(error: any): error is ArangoError { } /** - * Indicates whether the given value represents a {@link NetworkError}. + * Indicates whether the given value represents an ArangoDB error response. * - * @param error - A value that might be a `NetworkError`. - */ -export function isNetworkError(error: any): error is NetworkError { - return error instanceof NetworkError; -} - -/** * @internal -* - * Indicates whether the given value represents an ArangoDB error response. */ -export function isArangoErrorResponse(body: any): body is ArangoErrorResponse { +export function isArangoErrorResponse(body: any): boolean { return ( body && - body.error === true && - typeof body.code === 'number' && - typeof body.errorMessage === 'string' && - typeof body.errorNum === 'number' + body.hasOwnProperty("error") && + body.hasOwnProperty("code") && + body.hasOwnProperty("errorMessage") && + body.hasOwnProperty("errorNum") ); } /** - * @internal - * * Indicates whether the given value represents a Node.js `SystemError`. */ -function isSystemError(err: any): err is SystemError { +export function isSystemError(err: any): err is SystemError { return ( - err && Object.getPrototypeOf(err) === Error.prototype && - typeof err.code === 'string' && - typeof err.errno !== 'undefined' && - typeof err.syscall === 'string' - ); -} - -/** - * @internal - * - * Indicates whether the given value represents a Node.js `UndiciError`. - */ -function isUndiciError(err: any): err is UndiciError { - return ( - err && - err instanceof Error && - typeof (err as UndiciError).code === 'string' && - (err as UndiciError).code.startsWith('UND_') + err.hasOwnProperty("code") && + err.hasOwnProperty("errno") && + err.hasOwnProperty("syscall") ); } -/** - * @internal - * - * Determines whether the given failed fetch error cause is safe to retry. - */ -function isSafeToRetryFailedFetch(cause: Error): boolean | null { - if (isSystemError(cause) && cause.syscall === 'connect' && cause.code === 'ECONNREFUSED') { - return true; - } - if (isUndiciError(cause) && cause.code === 'UND_ERR_CONNECT_TIMEOUT') { - return true; - } - return null; -} - -/** - * @internal -* - * Interface representing an ArangoDB error response. - */ -export interface ArangoErrorResponse { - error: true; - code: number; - errorMessage: string; - errorNum: number; -} - -/** - * Interface representing a Node.js `UndiciError`. - * - * @internal - */ -interface UndiciError extends Error { - code: `UND_${string}`; -} - /** * Interface representing a Node.js `SystemError`. - * - * @internal */ -interface SystemError extends Error { +export interface SystemError extends Error { code: string; errno: number | string; syscall: string; } /** - * Represents an error from a deliberate timeout encountered while waiting - * for propagation. - */ -export class PropagationTimeoutError extends Error { - name = "PropagationTimeoutError"; - - constructor(message: string | undefined, options: { cause: Error }) { - super(message ?? 'Timed out while waiting for propagation', options); - } -} - -/** - * Represents a network error or an error encountered while performing a network request. + * Represents an error returned by ArangoDB. */ -export class NetworkError extends Error { - name = "NetworkError"; - - /** - * Indicates whether the request that caused this error can be safely retried. - */ - isSafeToRetry: boolean | null; - +export class ArangoError extends Error { + name = "ArangoError"; /** - * Fetch request object. + * ArangoDB error code. + * + * See [ArangoDB error documentation](https://www.arangodb.com/docs/stable/appendix-error-codes.html). */ - request: globalThis.Request; - - constructor(message: string, options: { request: globalThis.Request, cause?: Error, isSafeToRetry?: boolean | null }) { - const { request, isSafeToRetry = null, ...opts } = options; - super(message, opts); - this.request = request; - this.isSafeToRetry = isSafeToRetry; - } - - toJSON() { - return { - error: true, - errorMessage: this.message, - code: 0, - }; - } -} - -/** - * Represents an error from a deliberate timeout encountered while waiting - * for a server response. - */ -export class ResponseTimeoutError extends NetworkError { - name = "ResponseTimeoutError"; - - constructor(message: string | undefined, options: { request: globalThis.Request, cause?: Error, isSafeToRetry?: boolean | null }) { - super(message ?? 'Timed out while waiting for server response', options); - } -} - -/** - * Represents an error from a request that was aborted. - */ -export class RequestAbortedError extends NetworkError { - name = "RequestAbortedError"; - - constructor(message: string | undefined, options: { request: globalThis.Request, cause?: Error, isSafeToRetry?: boolean | null }) { - super(message ?? 'Request aborted', options); - } -} - -/** - * Represents an error from a failed fetch request. - * - * The root cause is often extremely difficult to determine. - */ -export class FetchFailedError extends NetworkError { - name = "FetchFailedError"; - - constructor(message: string | undefined, options: { request: globalThis.Request, cause: TypeError, isSafeToRetry?: boolean | null }) { - let isSafeToRetry = options.isSafeToRetry; - if (options.cause.cause instanceof Error) { - if (isSafeToRetry === undefined) { - isSafeToRetry = isSafeToRetryFailedFetch(options.cause.cause) || undefined; - } - if (message === undefined) { - message = `Fetch failed: ${options.cause.cause.message}`; - } - } - super(message ?? 'Fetch failed', { ...options, isSafeToRetry }); - } -} - -/** - * Represents a plain HTTP error response. - */ -export class HttpError extends NetworkError { - name = "HttpError"; - + errorNum: number; /** - * HTTP status code of the server response. + * HTTP status code included in the server error response object. */ code: number; - /** * Server response object. */ - response: ProcessedResponse; + response: any; /** * @internal */ - constructor(response: ProcessedResponse, options: { cause?: Error, isSafeToRetry?: boolean | null } = {}) { - const message = messages[response.status] ?? messages[500]; - super(message, { ...options, request: response.request }); + constructor(response: ArangojsResponse) { + super(); this.response = response; - this.code = response.status; + this.message = response.parsedBody.errorMessage; + this.errorNum = response.parsedBody.errorNum; + this.code = response.parsedBody.code; + const err = new Error(this.message); + err.name = this.name; + for (const key of nativeErrorKeys) { + if (err[key]) this[key] = err[key] as string; + } + } + + /** + * @internal + * + * Indicates that this object represents an ArangoDB error. + */ + get isArangoError(): true { + return true; } toJSON() { return { error: true, errorMessage: this.message, + errorNum: this.errorNum, code: this.code, }; } } /** - * Represents an error returned by ArangoDB. + * Represents a plain HTTP error response. */ -export class ArangoError extends Error { - name = "ArangoError"; - - /** - * Indicates whether the request that caused this error can be safely retried. - * - * @internal - */ - isSafeToRetry: boolean | null = null; - - /** - * ArangoDB error code. - * - * See [ArangoDB error documentation](https://www.arangodb.com/docs/stable/appendix-error-codes.html). - */ - errorNum: number; - +export class HttpError extends Error { + name = "HttpError"; /** - * Error message accompanying the error code. + * Server response object. */ - get errorMessage(): string { - return this.message; - } - + response: any; /** - * HTTP status code included in the server error response object. + * HTTP status code of the server response. */ code: number; /** * @internal - * - * Creates a new `ArangoError` from a response object. - */ - static from(response: ProcessedResponse): ArangoError { - return new ArangoError(response.parsedBody!, { - cause: new HttpError(response) - }); - } - - /** - * Creates a new `ArangoError` from an ArangoDB error response. - */ - constructor(data: ArangoErrorResponse, options: { cause?: Error, isSafeToRetry?: boolean | null }) { - const { isSafeToRetry, ...opts } = options; - super(data.errorMessage, opts); - this.errorNum = data.errorNum; - this.code = data.code; - if (isSafeToRetry !== undefined) { - this.isSafeToRetry = isSafeToRetry; - } else if (this.errorNum === ERROR_ARANGO_MAINTENANCE_MODE) { - this.isSafeToRetry = true; - } else if (this.cause instanceof NetworkError) { - this.isSafeToRetry = this.cause.isSafeToRetry; - } - } - - /** - * Server response object. - */ - get response(): ProcessedResponse | undefined { - const cause = this.cause; - if (cause instanceof HttpError) { - return cause.response; - } - return undefined; - } - - /** - * Fetch request object. */ - get request(): globalThis.Request | undefined { - const cause = this.cause; - if (cause instanceof NetworkError) { - return cause.request; + constructor(response: ArangojsResponse) { + super(); + this.response = response; + this.code = response.status || 500; + this.message = messages[this.code] || messages[500]; + const err = new Error(this.message); + err.name = this.name; + for (const key of nativeErrorKeys) { + if (err[key]) this[key] = err[key] as string; } - return undefined; } - /** - * @internal - * - * Indicates that this object represents an ArangoDB error. - */ - get isArangoError(): true { - return true; - } - - toJSON(): ArangoErrorResponse { + toJSON() { return { error: true, - errorMessage: this.errorMessage, - errorNum: this.errorNum, code: this.code, }; } -} \ No newline at end of file +} diff --git a/src/job.ts b/src/job.ts index 92c2ada7..86e53ab6 100644 --- a/src/job.ts +++ b/src/job.ts @@ -1,5 +1,5 @@ -import { ProcessedResponse } from "./connection.js"; import { Database } from "./database.js"; +import { ArangojsResponse } from "./lib/request.js"; /** * Represents an async job in a {@link database.Database}. @@ -7,7 +7,7 @@ import { Database } from "./database.js"; export class Job { protected _id: string; protected _db: Database; - protected _transformResponse?: (res: ProcessedResponse) => Promise; + protected _transformResponse?: (res: ArangojsResponse) => Promise; protected _transformError?: (error: any) => Promise; protected _loaded: boolean = false; protected _result: T | undefined; @@ -18,7 +18,7 @@ export class Job { constructor( db: Database, id: string, - transformResponse?: (res: ProcessedResponse) => Promise, + transformResponse?: (res: ArangojsResponse) => Promise, transformError?: (error: any) => Promise ) { this._db = db; @@ -73,7 +73,7 @@ export class Job { */ async load(): Promise { if (!this.isLoaded) { - let res: ProcessedResponse; + let res: ArangojsResponse; try { res = await this._db.request( { diff --git a/src/lib/request.ts b/src/lib/request.ts index 50daed6d..0b421ac8 100644 --- a/src/lib/request.ts +++ b/src/lib/request.ts @@ -5,14 +5,36 @@ * @internal */ -import { FetchFailedError, NetworkError, RequestAbortedError, ResponseTimeoutError } from "../error.js"; +import { SystemError } from "../error.js"; -function timer(timeout: number, cb: () => void) { - const t = setTimeout(cb, timeout); - return () => clearTimeout(t); +/** + * @internal + */ +function systemErrorToJSON(this: SystemError) { + return { + error: true, + errno: this.errno, + code: this.code, + syscall: this.syscall, + }; +} + +/** + * @internal + */ +export interface ArangojsResponse extends globalThis.Response { + request: globalThis.Request; + parsedBody?: any; + arangojsHostUrl?: string; } -export const REASON_TIMEOUT = 'timeout'; +/** + * @internal + */ +export interface ArangojsError extends Error { + request: globalThis.Request; + toJSON: () => Record; +} /** * @internal @@ -33,15 +55,15 @@ export type RequestOptions = { export type RequestConfig = { credentials: "omit" | "include" | "same-origin"; keepalive: boolean; - beforeRequest?: (req: globalThis.Request) => void | Promise; - afterResponse?: (err: NetworkError | null, res?: globalThis.Response & { request: globalThis.Request }) => void | Promise; + beforeRequest?: (req: globalThis.Request) => void; + afterResponse?: (err: ArangojsError | null, res?: ArangojsResponse) => void; }; /** * @internal */ export type RequestFunction = { - (options: RequestOptions): Promise; + (options: RequestOptions): Promise; close?: () => void; }; @@ -63,7 +85,7 @@ export function createRequest( baseUrl: URL, config: RequestConfig ): RequestFunction { - let abort: () => void | undefined; + let abort: AbortController | undefined; return Object.assign( async function request({ method, @@ -106,54 +128,38 @@ export function createRequest( keepalive: config.keepalive, }); if (config.beforeRequest) { - const p = config.beforeRequest(request); - if (p instanceof Promise) await p; + config.beforeRequest(request); } - const abortController = new AbortController(); - const signal = abortController.signal; - abort = () => abortController.abort(); - let clearTimer: (() => void) | undefined; + abort = new AbortController(); + let t: ReturnType | undefined; if (timeout) { - clearTimer = timer(timeout, () => { - clearTimer = undefined; - abortController.abort(REASON_TIMEOUT); - }); + t = setTimeout(() => { + abort?.abort(); + }, timeout); } - let response: globalThis.Response & { request: globalThis.Request }; try { - response = Object.assign(await fetch(request, { signal }), { request }); - } catch (e: unknown) { - const cause = e instanceof Error ? e : new Error(String(e)); - let error: NetworkError; - if (signal.aborted) { - const reason = typeof signal.reason == 'string' ? signal.reason : undefined; - if (reason === REASON_TIMEOUT) { - error = new ResponseTimeoutError(undefined, { request }); - } else { - error = new RequestAbortedError(reason, { request, cause }); - } - } else if (cause instanceof TypeError) { - error = new FetchFailedError(undefined, { request, cause }); - } else { - error = new NetworkError(cause.message, { request, cause }); + const res = await fetch(request, { signal: abort.signal }); + if (t) clearTimeout(t); + const response = res as ArangojsResponse; + response.request = request; + if (config.afterResponse) { + config.afterResponse(null, response); } + return response; + } catch (err) { + if (t) clearTimeout(t); + const error = err as ArangojsError; + error.request = request; + error.toJSON = systemErrorToJSON; if (config.afterResponse) { - const p = config.afterResponse(error); - if (p instanceof Promise) await p; + config.afterResponse(error); } throw error; - } finally { - clearTimer?.(); - } - if (config.afterResponse) { - const p = config.afterResponse(null, response); - if (p instanceof Promise) await p; } - return response; }, { close() { - abort?.(); + abort?.abort(); }, } ); diff --git a/src/route.ts b/src/route.ts index fe52bfce..fef1954a 100644 --- a/src/route.ts +++ b/src/route.ts @@ -7,8 +7,9 @@ * * @packageDocumentation */ -import { ProcessedResponse, RequestOptions } from "./connection.js"; +import { RequestOptions } from "./connection.js"; import { Database } from "./database.js"; +import { ArangojsResponse } from "./lib/request.js"; import { mergeHeaders } from "./lib/mergeHeaders.js"; /** @@ -129,7 +130,7 @@ export class Route { path: string, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; + ): Promise; /** * Performs a DELETE request against the given path relative to this route * and returns the server response. @@ -148,8 +149,8 @@ export class Route { delete( search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; - delete(...args: any[]): Promise { + ): Promise; + delete(...args: any[]): Promise { const path = typeof args[0] === "string" ? args.shift() : undefined; const [search, headers] = args; return this.request({ method: "DELETE", path, search, headers }); @@ -174,7 +175,7 @@ export class Route { path: string, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; + ): Promise; /** * Performs a GET request against the given path relative to this route * and returns the server response. @@ -193,8 +194,8 @@ export class Route { get( search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; - get(...args: any[]): Promise { + ): Promise; + get(...args: any[]): Promise { const path = typeof args[0] === "string" ? args.shift() : undefined; const [search, headers] = args; return this.request({ method: "GET", path, search, headers }); @@ -219,7 +220,7 @@ export class Route { path: string, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; + ): Promise; /** * Performs a HEAD request against the given path relative to this route * and returns the server response. @@ -238,8 +239,8 @@ export class Route { head( search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; - head(...args: any[]): Promise { + ): Promise; + head(...args: any[]): Promise { const path = typeof args[0] === "string" ? args.shift() : undefined; const [search, headers] = args; return this.request({ method: "HEAD", path, search, headers }); @@ -266,7 +267,7 @@ export class Route { body?: any, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; + ): Promise; /** * Performs a PATCH request against the given path relative to this route * and returns the server response. @@ -289,8 +290,8 @@ export class Route { body?: any, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; - patch(...args: any[]): Promise { + ): Promise; + patch(...args: any[]): Promise { const path = typeof args[0] === "string" ? args.shift() : undefined; const [body, search, headers] = args; return this.request({ method: "PATCH", path, body, search, headers }); @@ -320,7 +321,7 @@ export class Route { body?: any, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; + ): Promise; /** * Performs a POST request against the given path relative to this route * and returns the server response. @@ -346,8 +347,8 @@ export class Route { body?: any, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; - post(...args: any[]): Promise { + ): Promise; + post(...args: any[]): Promise { const path = typeof args[0] === "string" ? args.shift() : undefined; const [body, search, headers] = args; return this.request({ method: "POST", path, body, search, headers }); @@ -374,7 +375,7 @@ export class Route { body?: any, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; + ): Promise; /** * Performs a PUT request against the given path relative to this route * and returns the server response. @@ -397,8 +398,8 @@ export class Route { body?: any, search?: URLSearchParams | Record, headers?: Headers | Record - ): Promise; - put(...args: any[]): Promise { + ): Promise; + put(...args: any[]): Promise { const path = typeof args[0] === "string" ? args.shift() : undefined; const [body, search, headers] = args; return this.request({ method: "PUT", path, body, search, headers }); diff --git a/src/test/27-query-management.ts b/src/test/27-query-management.ts index 8cf1b3d9..b1e6bda2 100644 --- a/src/test/27-query-management.ts +++ b/src/test/27-query-management.ts @@ -2,7 +2,7 @@ import { expect } from "chai"; import { aql } from "../aql.js"; import { ArrayCursor } from "../cursor.js"; import { Database } from "../database.js"; -import { ArangoError, ResponseTimeoutError } from "../error.js"; +import { ArangoError } from "../error.js"; import { config } from "./_config.js"; // NOTE These tests will not reliably work with load balancing. @@ -67,7 +67,7 @@ describe("Query Management API", function () { } catch (err: any) { expect(err).is.instanceof(Error); expect(err).is.not.instanceof(ArangoError); - expect(err).is.instanceof(ResponseTimeoutError); + expect(err.name).to.equal("AbortError"); return; } expect.fail();