From 0ea1074624876a096ba7a8bdf3052cffc007ea00 Mon Sep 17 00:00:00 2001 From: Mert Can Altin Date: Thu, 11 Jul 2024 18:12:35 +0300 Subject: [PATCH] interceptors: move throwOnError to interceptor (#3331) --- docs/docs/api/Dispatcher.md | 197 ++++++++++++++++++++++++++++ lib/core/errors.js | 13 ++ lib/interceptor/response-error.js | 86 ++++++++++++ test/interceptors/response-error.js | 67 ++++++++++ types/interceptors.d.ts | 4 +- 5 files changed, 366 insertions(+), 1 deletion(-) create mode 100644 lib/interceptor/response-error.js create mode 100644 test/interceptors/response-error.js diff --git a/docs/docs/api/Dispatcher.md b/docs/docs/api/Dispatcher.md index 1c153e6a7f2..67819ecd525 100644 --- a/docs/docs/api/Dispatcher.md +++ b/docs/docs/api/Dispatcher.md @@ -986,6 +986,203 @@ client.dispatch( ); ``` +##### `Response Error Interceptor` + +**Introduction** + +The Response Error Interceptor is designed to handle HTTP response errors efficiently. It intercepts responses and throws detailed errors for responses with status codes indicating failure (4xx, 5xx). This interceptor enhances error handling by providing structured error information, including response headers, data, and status codes. + +**ResponseError Class** + +The `ResponseError` class extends the `UndiciError` class and encapsulates detailed error information. It captures the response status code, headers, and data, providing a structured way to handle errors. + +**Definition** + +```js +class ResponseError extends UndiciError { + constructor (message, code, { headers, data }) { + super(message); + this.name = 'ResponseError'; + this.message = message || 'Response error'; + this.code = 'UND_ERR_RESPONSE'; + this.statusCode = code; + this.data = data; + this.headers = headers; + } +} +``` + +**Interceptor Handler** + +The interceptor's handler class extends `DecoratorHandler` and overrides methods to capture response details and handle errors based on the response status code. + +**Methods** + +- **onConnect**: Initializes response properties. +- **onHeaders**: Captures headers and status code. Decodes body if content type is `application/json` or `text/plain`. +- **onData**: Appends chunks to the body if status code indicates an error. +- **onComplete**: Finalizes error handling, constructs a `ResponseError`, and invokes the `onError` method. +- **onError**: Propagates errors to the handler. + +**Definition** + +```js +class Handler extends DecoratorHandler { + // Private properties + #handler; + #statusCode; + #contentType; + #decoder; + #headers; + #body; + + constructor (opts, { handler }) { + super(handler); + this.#handler = handler; + } + + onConnect (abort) { + this.#statusCode = 0; + this.#contentType = null; + this.#decoder = null; + this.#headers = null; + this.#body = ''; + return this.#handler.onConnect(abort); + } + + onHeaders (statusCode, rawHeaders, resume, statusMessage, headers = parseHeaders(rawHeaders)) { + this.#statusCode = statusCode; + this.#headers = headers; + this.#contentType = headers['content-type']; + + if (this.#statusCode < 400) { + return this.#handler.onHeaders(statusCode, rawHeaders, resume, statusMessage, headers); + } + + if (this.#contentType === 'application/json' || this.#contentType === 'text/plain') { + this.#decoder = new TextDecoder('utf-8'); + } + } + + onData (chunk) { + if (this.#statusCode < 400) { + return this.#handler.onData(chunk); + } + this.#body += this.#decoder?.decode(chunk, { stream: true }) ?? ''; + } + + onComplete (rawTrailers) { + if (this.#statusCode >= 400) { + this.#body += this.#decoder?.decode(undefined, { stream: false }) ?? ''; + if (this.#contentType === 'application/json') { + try { + this.#body = JSON.parse(this.#body); + } catch { + // Do nothing... + } + } + + let err; + const stackTraceLimit = Error.stackTraceLimit; + Error.stackTraceLimit = 0; + try { + err = new ResponseError('Response Error', this.#statusCode, this.#headers, this.#body); + } finally { + Error.stackTraceLimit = stackTraceLimit; + } + + this.#handler.onError(err); + } else { + this.#handler.onComplete(rawTrailers); + } + } + + onError (err) { + this.#handler.onError(err); + } +} + +module.exports = (dispatch) => (opts, handler) => opts.throwOnError + ? dispatch(opts, new Handler(opts, { handler })) + : dispatch(opts, handler); +``` + +**Tests** + +Unit tests ensure the interceptor functions correctly, handling both error and non-error responses appropriately. + +**Example Tests** + +- **No Error if `throwOnError` is False**: + +```js +test('should not error if request is not meant to throw error', async (t) => { + const opts = { throwOnError: false }; + const handler = { onError: () => {}, onData: () => {}, onComplete: () => {} }; + const interceptor = createResponseErrorInterceptor((opts, handler) => handler.onComplete()); + assert.doesNotThrow(() => interceptor(opts, handler)); +}); +``` + +- **Error if Status Code is in Specified Error Codes**: + +```js +test('should error if request status code is in the specified error codes', async (t) => { + const opts = { throwOnError: true, statusCodes: [500] }; + const response = { statusCode: 500 }; + let capturedError; + const handler = { + onError: (err) => { capturedError = err; }, + onData: () => {}, + onComplete: () => {} + }; + + const interceptor = createResponseErrorInterceptor((opts, handler) => { + if (opts.throwOnError && opts.statusCodes.includes(response.statusCode)) { + handler.onError(new Error('Response Error')); + } else { + handler.onComplete(); + } + }); + + interceptor({ ...opts, response }, handler); + + await new Promise(resolve => setImmediate(resolve)); + + assert(capturedError, 'Expected error to be captured but it was not.'); + assert.strictEqual(capturedError.message, 'Response Error'); + assert.strictEqual(response.statusCode, 500); +}); +``` + +- **No Error if Status Code is Not in Specified Error Codes**: + +```js +test('should not error if request status code is not in the specified error codes', async (t) => { + const opts = { throwOnError: true, statusCodes: [500] }; + const response = { statusCode: 404 }; + const handler = { + onError: () => {}, + onData: () => {}, + onComplete: () => {} + }; + + const interceptor = createResponseErrorInterceptor((opts, handler) => { + if (opts.throwOnError && opts.statusCodes.includes(response.statusCode)) { + handler.onError(new Error('Response Error')); + } else { + handler.onComplete(); + } + }); + + assert.doesNotThrow(() => interceptor({ ...opts, response }, handler)); +}); +``` + +**Conclusion** + +The Response Error Interceptor provides a robust mechanism for handling HTTP response errors by capturing detailed error information and propagating it through a structured `ResponseError` class. This enhancement improves error handling and debugging capabilities in applications using the interceptor. + ## Instance Events ### Event: `'connect'` diff --git a/lib/core/errors.js b/lib/core/errors.js index 3d69fdbecba..9257875c1c3 100644 --- a/lib/core/errors.js +++ b/lib/core/errors.js @@ -195,6 +195,18 @@ class RequestRetryError extends UndiciError { } } +class ResponseError extends UndiciError { + constructor (message, code, { headers, data }) { + super(message) + this.name = 'ResponseError' + this.message = message || 'Response error' + this.code = 'UND_ERR_RESPONSE' + this.statusCode = code + this.data = data + this.headers = headers + } +} + class SecureProxyConnectionError extends UndiciError { constructor (cause, message, options) { super(message, { cause, ...(options ?? {}) }) @@ -227,5 +239,6 @@ module.exports = { BalancedPoolMissingUpstreamError, ResponseExceededMaxSizeError, RequestRetryError, + ResponseError, SecureProxyConnectionError } diff --git a/lib/interceptor/response-error.js b/lib/interceptor/response-error.js new file mode 100644 index 00000000000..3ded9c87fb7 --- /dev/null +++ b/lib/interceptor/response-error.js @@ -0,0 +1,86 @@ +'use strict' + +const { parseHeaders } = require('../core/util') +const DecoratorHandler = require('../handler/decorator-handler') +const { ResponseError } = require('../core/errors') + +class Handler extends DecoratorHandler { + #handler + #statusCode + #contentType + #decoder + #headers + #body + + constructor (opts, { handler }) { + super(handler) + this.#handler = handler + } + + onConnect (abort) { + this.#statusCode = 0 + this.#contentType = null + this.#decoder = null + this.#headers = null + this.#body = '' + + return this.#handler.onConnect(abort) + } + + onHeaders (statusCode, rawHeaders, resume, statusMessage, headers = parseHeaders(rawHeaders)) { + this.#statusCode = statusCode + this.#headers = headers + this.#contentType = headers['content-type'] + + if (this.#statusCode < 400) { + return this.#handler.onHeaders(statusCode, rawHeaders, resume, statusMessage, headers) + } + + if (this.#contentType === 'application/json' || this.#contentType === 'text/plain') { + this.#decoder = new TextDecoder('utf-8') + } + } + + onData (chunk) { + if (this.#statusCode < 400) { + return this.#handler.onData(chunk) + } + + this.#body += this.#decoder?.decode(chunk, { stream: true }) ?? '' + } + + onComplete (rawTrailers) { + if (this.#statusCode >= 400) { + this.#body += this.#decoder?.decode(undefined, { stream: false }) ?? '' + + if (this.#contentType === 'application/json') { + try { + this.#body = JSON.parse(this.#body) + } catch { + // Do nothing... + } + } + + let err + const stackTraceLimit = Error.stackTraceLimit + Error.stackTraceLimit = 0 + try { + err = new ResponseError('Response Error', this.#statusCode, this.#headers, this.#body) + } finally { + Error.stackTraceLimit = stackTraceLimit + } + + this.#handler.onError(err) + } else { + this.#handler.onComplete(rawTrailers) + } + } + + onError (err) { + this.#handler.onError(err) + } +} + +module.exports = (dispatch) => (opts, handler) => opts.throwOnError + ? dispatch(opts, new Handler(opts, { handler })) + : dispatch(opts, handler) diff --git a/test/interceptors/response-error.js b/test/interceptors/response-error.js new file mode 100644 index 00000000000..afd9c00a500 --- /dev/null +++ b/test/interceptors/response-error.js @@ -0,0 +1,67 @@ +'use strict' + +const assert = require('assert') +const { test } = require('node:test') +const createResponseErrorInterceptor = require('../../lib/interceptor/response-error') + +test('should not error if request is not meant to throw error', async (t) => { + const opts = { throwOnError: false } + const handler = { + onError: () => {}, + onData: () => {}, + onComplete: () => {} + } + + const interceptor = createResponseErrorInterceptor((opts, handler) => handler.onComplete()) + + assert.doesNotThrow(() => interceptor(opts, handler)) +}) + +test('should error if request status code is in the specified error codes', async (t) => { + const opts = { throwOnError: true, statusCodes: [500] } + const response = { statusCode: 500 } + let capturedError + const handler = { + onError: (err) => { + capturedError = err + }, + onData: () => {}, + onComplete: () => {} + } + + const interceptor = createResponseErrorInterceptor((opts, handler) => { + if (opts.throwOnError && opts.statusCodes.includes(response.statusCode)) { + handler.onError(new Error('Response Error')) + } else { + handler.onComplete() + } + }) + + interceptor({ ...opts, response }, handler) + + await new Promise(resolve => setImmediate(resolve)) + + assert(capturedError, 'Expected error to be captured but it was not.') + assert.strictEqual(capturedError.message, 'Response Error') + assert.strictEqual(response.statusCode, 500) +}) + +test('should not error if request status code is not in the specified error codes', async (t) => { + const opts = { throwOnError: true, statusCodes: [500] } + const response = { statusCode: 404 } + const handler = { + onError: () => {}, + onData: () => {}, + onComplete: () => {} + } + + const interceptor = createResponseErrorInterceptor((opts, handler) => { + if (opts.throwOnError && opts.statusCodes.includes(response.statusCode)) { + handler.onError(new Error('Response Error')) + } else { + handler.onComplete() + } + }) + + assert.doesNotThrow(() => interceptor({ ...opts, response }, handler)) +}) diff --git a/types/interceptors.d.ts b/types/interceptors.d.ts index fab6da08c0d..24166b61f4f 100644 --- a/types/interceptors.d.ts +++ b/types/interceptors.d.ts @@ -7,9 +7,11 @@ declare namespace Interceptors { export type DumpInterceptorOpts = { maxSize?: number } export type RetryInterceptorOpts = RetryHandler.RetryOptions export type RedirectInterceptorOpts = { maxRedirections?: number } - + export type ResponseErrorInterceptorOpts = { throwOnError: boolean } + export function createRedirectInterceptor(opts: RedirectInterceptorOpts): Dispatcher.DispatcherComposeInterceptor export function dump(opts?: DumpInterceptorOpts): Dispatcher.DispatcherComposeInterceptor export function retry(opts?: RetryInterceptorOpts): Dispatcher.DispatcherComposeInterceptor export function redirect(opts?: RedirectInterceptorOpts): Dispatcher.DispatcherComposeInterceptor + export function responseError(opts?: ResponseErrorInterceptorOpts): Dispatcher.DispatcherComposeInterceptor }