Skip to content

Commit

Permalink
fix: response error interceptor broken (#3805)
Browse files Browse the repository at this point in the history
* fix: response error interceptor broken

Expose the interceptor in the same manner as the others.

Remove `throwOnError` as it is considered an invalid argument.

* cleanup

* docs

---------

Co-authored-by: Matteo Collina <[email protected]>
  • Loading branch information
luddd3 and mcollina authored Nov 27, 2024
1 parent 20fd58c commit de6aea6
Show file tree
Hide file tree
Showing 4 changed files with 88 additions and 242 deletions.
200 changes: 12 additions & 188 deletions docs/docs/api/Dispatcher.md
Original file line number Diff line number Diff line change
Expand Up @@ -1054,203 +1054,27 @@ const response = await client.request({
})
```

##### `Response Error Interceptor`
##### `responseError`

**Introduction**
The `responseError` interceptor throws an error for responses with status code errors (>= 400).

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**:
**Example**

```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**:
const { Client, interceptors } = require("undici");
const { responseError } = interceptors;

```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();
}
});
const client = new Client("http://example.com").compose(
responseError()
);

assert.doesNotThrow(() => interceptor({ ...opts, response }, handler));
// Will throw a ResponseError for status codes >= 400
await client.request({
method: "GET",
path: "/"
});
```

**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.
##### `Cache Interceptor`

The `cache` interceptor implements client-side response caching as described in
Expand Down
1 change: 1 addition & 0 deletions index.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ module.exports.DecoratorHandler = DecoratorHandler
module.exports.RedirectHandler = RedirectHandler
module.exports.interceptors = {
redirect: require('./lib/interceptor/redirect'),
responseError: require('./lib/interceptor/response-error'),
retry: require('./lib/interceptor/retry'),
dump: require('./lib/interceptor/dump'),
dns: require('./lib/interceptor/dns'),
Expand Down
12 changes: 8 additions & 4 deletions lib/interceptor/response-error.js
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ const { parseHeaders } = require('../core/util')
const DecoratorHandler = require('../handler/decorator-handler')
const { ResponseError } = require('../core/errors')

class Handler extends DecoratorHandler {
class ResponseErrorHandler extends DecoratorHandler {
#handler
#statusCode
#contentType
Expand Down Expand Up @@ -84,6 +84,10 @@ class Handler extends DecoratorHandler {
}
}

module.exports = (dispatch) => (opts, handler) => opts.throwOnError
? dispatch(opts, new Handler(opts, { handler }))
: dispatch(opts, handler)
module.exports = () => {
return (dispatch) => {
return function Intercept (opts, handler) {
return dispatch(opts, new ResponseErrorHandler(opts, { handler }))
}
}
}
117 changes: 67 additions & 50 deletions test/interceptors/response-error.js
Original file line number Diff line number Diff line change
@@ -1,67 +1,84 @@
'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 assert = require('node:assert')
const { once } = require('node:events')
const { createServer } = require('node:http')
const { test, after } = require('node:test')
const { interceptors, Client } = require('../..')
const { responseError } = interceptors

const interceptor = createResponseErrorInterceptor((opts, handler) => handler.onComplete())
test('should throw error for error response', async () => {
const server = createServer()

assert.doesNotThrow(() => interceptor(opts, handler))
})
server.on('request', (req, res) => {
res.writeHead(400, { 'content-type': 'text/plain' })
res.end('Bad Request')
})

server.listen(0)

await once(server, 'listening')

const client = new Client(
`http://localhost:${server.address().port}`
).compose(responseError())

after(async () => {
await client.close()
server.close()

await once(server, 'close')
})

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: () => {}
let error
try {
await client.request({
method: 'GET',
path: '/',
headers: {
'content-type': 'text/plain'
}
})
} catch (err) {
error = err
}

const interceptor = createResponseErrorInterceptor((opts, handler) => {
if (opts.throwOnError && opts.statusCodes.includes(response.statusCode)) {
handler.onError(new Error('Response Error'))
} else {
handler.onComplete()
}
assert.equal(error.statusCode, 400)
assert.equal(error.message, 'Response Error')
assert.equal(error.data, 'Bad Request')
})

test('should not throw error for ok response', async () => {
const server = createServer()

server.on('request', (req, res) => {
res.writeHead(200, { 'content-type': 'text/plain' })
res.end('hello')
})

interceptor({ ...opts, response }, handler)
server.listen(0)

await new Promise(resolve => setImmediate(resolve))
await once(server, 'listening')

assert(capturedError, 'Expected error to be captured but it was not.')
assert.strictEqual(capturedError.message, 'Response Error')
assert.strictEqual(response.statusCode, 500)
})
const client = new Client(
`http://localhost:${server.address().port}`
).compose(responseError())

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: () => {}
}
after(async () => {
await client.close()
server.close()

await once(server, 'close')
})

const interceptor = createResponseErrorInterceptor((opts, handler) => {
if (opts.throwOnError && opts.statusCodes.includes(response.statusCode)) {
handler.onError(new Error('Response Error'))
} else {
handler.onComplete()
const response = await client.request({
method: 'GET',
path: '/',
headers: {
'content-type': 'text/plain'
}
})

assert.doesNotThrow(() => interceptor({ ...opts, response }, handler))
assert.equal(response.statusCode, 200)
assert.equal(await response.body.text(), 'hello')
})

0 comments on commit de6aea6

Please sign in to comment.