feat: Option to throw on error status codes (nodejs#1453)

* Improve coverage

* Update TS types

* Fix linting

* Improve naming, add type tests

* Address code review comments

* make check explicit

Co-authored-by: Robert Nagy <[email protected]>

* make condition more explicit

Co-authored-by: Robert Nagy <[email protected]>

Co-authored-by: Robert Nagy <[email protected]>

Author: kibertoad

docs/api/Dispatcher.md

@@ -200,13 +200,14 @@ Returns: `Boolean` - `false` if dispatcher is busy and further dispatch calls wo
 * **upgrade** `string | null` (optional) - Default: `null` - Upgrade the request. Should be used to specify the kind of upgrade i.e. `'Websocket'`.
 * **bodyTimeout** `number | null` (optional) - The timeout after which a request will time out, in milliseconds. Monitors time between receiving body data. Use `0` to disable it entirely. Defaults to 30 seconds.
 * **headersTimeout** `number | null` (optional) - The amount of time the parser will wait to receive the complete HTTP headers. Defaults to 30 seconds.
+* **throwOnError** `boolean` (optional) - Default: `false` - Whether Undici should throw an error upon receiving a 4xx or 5xx response from the server.
 
 #### Parameter: `DispatchHandler`
 
 * **onConnect** `(abort: () => void, context: object) => void` - Invoked before request is dispatched on socket. May be invoked multiple times when a request is retried when the request at the head of the pipeline fails.
 * **onError** `(error: Error) => void` - Invoked when an error has occurred. May not throw.
 * **onUpgrade** `(statusCode: number, headers: Buffer[], socket: Duplex) => void` (optional) - Invoked when request is upgraded. Required if `DispatchOptions.upgrade` is defined or `DispatchOptions.method === 'CONNECT'`.
-* **onHeaders** `(statusCode: number, headers: Buffer[], resume: () => void) => boolean` - Invoked when statusCode and headers have been received. May be invoked multiple times due to 1xx informational headers. Not required for `upgrade` requests.
+* **onHeaders** `(statusCode: number, headers: Buffer[], resume: () => void, statusText: string) => boolean` - Invoked when statusCode and headers have been received. May be invoked multiple times due to 1xx informational headers. Not required for `upgrade` requests.
 * **onData** `(chunk: Buffer) => boolean` - Invoked when response payload data is received. Not required for `upgrade` requests.
 * **onComplete** `(trailers: Buffer[]) => void` - Invoked when response payload and trailers have been received and the request has completed. Not required for `upgrade` requests.
 * **onBodySent** `(chunk: string | Buffer | Uint8Array) => void` - Invoked when a body chunk is sent to the server. Not required. For a stream or iterable body this will be invoked for every chunk. For other body types, it will be invoked once after the body is sent.

lib/api/api-request.js

@@ -3,7 +3,8 @@
 const Readable = require('./readable')
 const {
   InvalidArgumentError,
-  RequestAbortedError
+  RequestAbortedError,
+  ResponseStatusCodeError
 } = require('../core/errors')
 const util = require('../core/util')
 const { AsyncResource } = require('async_hooks')
@@ -15,7 +16,7 @@ class RequestHandler extends AsyncResource {
       throw new InvalidArgumentError('invalid opts')
     }
 
-    const { signal, method, opaque, body, onInfo, responseHeaders } = opts
+    const { signal, method, opaque, body, onInfo, responseHeaders, throwOnError } = opts
 
     try {
       if (typeof callback !== 'function') {
@@ -51,6 +52,7 @@ class RequestHandler extends AsyncResource {
     this.trailers = {}
     this.context = null
     this.onInfo = onInfo || null
+    this.throwOnError = throwOnError
 
     if (util.isStream(body)) {
       body.on('error', (err) => {
@@ -70,7 +72,7 @@ class RequestHandler extends AsyncResource {
     this.context = context
   }
 
-  onHeaders (statusCode, rawHeaders, resume) {
+  onHeaders (statusCode, rawHeaders, resume, statusMessage) {
     const { callback, opaque, abort, context } = this
 
     if (statusCode < 200) {
@@ -89,6 +91,13 @@ class RequestHandler extends AsyncResource {
     const headers = this.responseHeaders === 'raw' ? util.parseRawHeaders(rawHeaders) : util.parseHeaders(rawHeaders)
 
     if (callback !== null) {
+      if (this.throwOnError && statusCode >= 400) {
+        this.runInAsyncScope(callback, null,
+          new ResponseStatusCodeError(`Response status code ${statusCode}${statusMessage ? `: ${statusMessage}` : ''}`, statusCode, headers)
+        )
+        return
+      }
+
       this.runInAsyncScope(callback, null, null, {
         statusCode,
         headers,

lib/core/errors.js

@@ -56,6 +56,19 @@ class BodyTimeoutError extends UndiciError {
   }
 }
 
+class ResponseStatusCodeError extends UndiciError {
+  constructor (message, statusCode, headers) {
+    super(message)
+    Error.captureStackTrace(this, ResponseStatusCodeError)
+    this.name = 'ResponseStatusCodeError'
+    this.message = message || 'Response Status Code Error'
+    this.code = 'UND_ERR_RESPONSE_STATUS_CODE'
+    this.status = statusCode
+    this.statusCode = statusCode
+    this.headers = headers
+  }
+}
+
 class InvalidArgumentError extends UndiciError {
   constructor (message) {
     super(message)
@@ -186,6 +199,7 @@ module.exports = {
   BodyTimeoutError,
   RequestContentLengthMismatchError,
   ConnectTimeoutError,
+  ResponseStatusCodeError,
   InvalidArgumentError,
   InvalidReturnValueError,
   RequestAbortedError,

lib/core/request.js

@@ -43,7 +43,8 @@ class Request {
     blocking,
     upgrade,
     headersTimeout,
-    bodyTimeout
+    bodyTimeout,
+    throwOnError
   }, handler) {
     if (typeof path !== 'string') {
       throw new InvalidArgumentError('path must be a string')
@@ -71,6 +72,8 @@ class Request {
 
     this.bodyTimeout = bodyTimeout
 
+    this.throwOnError = throwOnError === true
+
     this.method = method
 
     if (body == null) {

test/client.js

@@ -315,6 +315,72 @@ test('basic get with query params partially in path', (t) => {
   })
 })
 
+test('basic get returns 400 when configured to throw on errors (callback)', (t) => {
+  t.plan(6)
+
+  const server = createServer((req, res) => {
+    res.statusCode = 400
+    res.end('hello')
+  })
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, () => {
+    const client = new Client(`http://localhost:${server.address().port}`, {
+      keepAliveTimeout: 300e3
+    })
+    t.teardown(client.close.bind(client))
+
+    const signal = new EE()
+    client.request({
+      signal,
+      path: '/',
+      method: 'GET',
+      throwOnError: true
+    }, (err) => {
+      t.equal(err.message, 'Response status code 400: Bad Request')
+      t.equal(err.status, 400)
+      t.equal(err.statusCode, 400)
+      t.equal(err.headers.connection, 'keep-alive')
+      t.equal(err.headers['content-length'], '5')
+    })
+    t.equal(signal.listenerCount('abort'), 1)
+  })
+})
+
+test('basic get returns 400 when configured to throw on errors (promise)', (t) => {
+  t.plan(5)
+
+  const server = createServer((req, res) => {
+    res.writeHead(400, 'Invalid params', { 'content-type': 'text/plain' })
+    res.end('Invalid params')
+  })
+  t.teardown(server.close.bind(server))
+
+  server.listen(0, async () => {
+    const client = new Client(`http://localhost:${server.address().port}`, {
+      keepAliveTimeout: 300e3
+    })
+    t.teardown(client.close.bind(client))
+
+    const signal = new EE()
+    try {
+      await client.request({
+        signal,
+        path: '/',
+        method: 'GET',
+        throwOnError: true
+      })
+      t.fail('Should throw an error')
+    } catch (err) {
+      t.equal(err.message, 'Response status code 400: Invalid params')
+      t.equal(err.status, 400)
+      t.equal(err.statusCode, 400)
+      t.equal(err.headers.connection, 'keep-alive')
+      t.equal(err.headers['content-type'], 'text/plain')
+    }
+  })
+})
+
 test('basic head', (t) => {
   t.plan(14)
 

test/types/dispatcher.test-d.ts

@@ -26,6 +26,9 @@ expectAssignable<Dispatcher>(new Dispatcher())
 
   // request
   expectAssignable<Promise<Dispatcher.ResponseData>>(dispatcher.request({ origin: '', path: '', method: 'GET', maxRedirections: 0 }))
+  expectAssignable<Promise<Dispatcher.ResponseData>>(dispatcher.request({ origin: '', path: '', method: 'GET', maxRedirections: 0, query: {} }))
+  expectAssignable<Promise<Dispatcher.ResponseData>>(dispatcher.request({ origin: '', path: '', method: 'GET', maxRedirections: 0, query: { pageNum: 1, id: 'abc' } }))
+  expectAssignable<Promise<Dispatcher.ResponseData>>(dispatcher.request({ origin: '', path: '', method: 'GET', maxRedirections: 0, throwOnError: true }))
   expectAssignable<Promise<Dispatcher.ResponseData>>(dispatcher.request({ origin: new URL('http://localhost'), path: '', method: 'GET' }))
   expectAssignable<void>(dispatcher.request({ origin: '', path: '', method: 'GET' }, (err, data) => {
     expectAssignable<Error | null>(err)

types/dispatcher.d.ts

@@ -57,6 +57,8 @@ declare namespace Dispatcher {
     headersTimeout?: number | null;
     /** The timeout after which a request will time out, in milliseconds. Monitors time between receiving body data. Use 0 to disable it entirely. Defaults to 30 seconds. */
     bodyTimeout?: number | null;
+    /** Whether Undici should throw an error upon receiving a 4xx or 5xx response from the server. Defaults to false */
+    throwOnError?: boolean;
   }
   export interface ConnectOptions {
     path: string;