Skip to content

Commit

Permalink
abort request with reason if one is provided (#2592)
Browse files Browse the repository at this point in the history
  • Loading branch information
KhafraDev authored Jan 5, 2024
1 parent 62efdad commit 81e3d84
Show file tree
Hide file tree
Showing 5 changed files with 49 additions and 14 deletions.
4 changes: 2 additions & 2 deletions lib/api/abort-signal.js
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@ const kSignal = Symbol('kSignal')

function abort (self) {
if (self.abort) {
self.abort()
self.abort(self[kSignal]?.reason)
} else {
self.onError(new RequestAbortedError())
self.onError(self[kSignal]?.reason ?? new RequestAbortedError())
}
}

Expand Down
6 changes: 3 additions & 3 deletions test/issue-2349.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@

const { test } = require('tap')
const { Writable } = require('stream')
const { MockAgent, errors, stream } = require('..')
const { MockAgent, stream } = require('..')

test('stream() does not fail after request has been aborted', async (t) => {
t.plan(1)
Expand All @@ -21,7 +21,7 @@ test('stream() does not fail after request has been aborted', async (t) => {
const parts = []
const ac = new AbortController()

setTimeout(() => ac.abort('nevermind'), 5)
setTimeout(() => ac.abort(), 5)

try {
await stream(
Expand All @@ -42,6 +42,6 @@ test('stream() does not fail after request has been aborted', async (t) => {
)
} catch (error) {
console.log(error)
t.equal(error instanceof errors.RequestAbortedError, true)
t.equal(error instanceof DOMException, true)
}
})
35 changes: 35 additions & 0 deletions test/issue-2590.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
'use strict'

const { test } = require('tap')
const { request } = require('..')
const { createServer } = require('http')
const { once } = require('events')

test('aborting request with custom reason', async (t) => {
const server = createServer(() => {}).listen(0)

t.teardown(server.close.bind(server))
await once(server, 'listening')

const timeout = AbortSignal.timeout(0)
const ac = new AbortController()
ac.abort(new Error('aborted'))

const ac2 = new AbortController()
ac2.abort() // no reason

await t.rejects(
request(`http://localhost:${server.address().port}`, { signal: timeout }),
timeout.reason
)

await t.rejects(
request(`http://localhost:${server.address().port}`, { signal: ac.signal }),
ac.signal.reason
)

await t.rejects(
request(`http://localhost:${server.address().port}`, { signal: ac2.signal }),
{ code: 'UND_ERR_ABORTED' }
)
})
2 changes: 1 addition & 1 deletion test/jest/mock-scope.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,5 +25,5 @@ it('Jest works with MockScope.delay - issue #1327', async () => {
signal: ac.signal
})

await expect(promise).rejects.toThrowError('Request aborted')
await expect(promise).rejects.toThrowError('This operation was aborted')
}, 1000)
16 changes: 8 additions & 8 deletions test/node-test/abort-controller.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
abortController.abort()

client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})

Expand Down Expand Up @@ -73,7 +73,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
})

client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})

abortController.abort()
Expand All @@ -98,7 +98,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
t.after(client.destroy.bind(client))

client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})

Expand All @@ -122,7 +122,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
t.after(client.destroy.bind(client))

client.request({ path: '/', method: 'GET', signal: abortController.signal }, (err, response) => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})

Expand All @@ -149,7 +149,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
abortController.abort()
})
response.body.on('error', err => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})
})
Expand All @@ -174,7 +174,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
t.after(client.destroy.bind(client))

client.request({ path: '/', method: 'POST', body, signal: abortController.signal }, (err, response) => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})
await p.completed
Expand Down Expand Up @@ -204,7 +204,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
t.after(client.destroy.bind(client))

client.request({ path: '/', method: 'POST', body, signal: abortController.signal }, (err, response) => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})
await p.completed
Expand Down Expand Up @@ -237,7 +237,7 @@ for (const { AbortControllerImpl, controllerName } of controllers) {
abortController.abort()
})
response.body.on('error', err => {
p.ok(err instanceof errors.RequestAbortedError)
p.ok(err instanceof errors.RequestAbortedError || err instanceof DOMException)
})
})
})
Expand Down

0 comments on commit 81e3d84

Please sign in to comment.