Skip to content

Commit 7c08fe0

Browse files
authored
feat: inherit abort signal on piping (google#992)
* feat: inherit abort signal on piping closes google#968 * test: add delay for nc init * test: get free port to avoid net collisions * test: fix flaky test * test(cli): replace `nc` with `node:net`
1 parent 3841cbe commit 7c08fe0

File tree

5 files changed

+62
-9
lines changed

5 files changed

+62
-9
lines changed

package-lock.json

+14
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

package.json

+1
Original file line numberDiff line numberDiff line change
@@ -114,6 +114,7 @@
114114
"esbuild-plugin-transform-hook": "^0.1.1",
115115
"esbuild-plugin-utils": "^0.1.0",
116116
"fs-extra": "^11.2.0",
117+
"get-port": "^7.1.0",
117118
"globby": "^14.0.2",
118119
"jsr": "^0.13.2",
119120
"madge": "^8.0.0",

src/core.ts

+8-1
Original file line numberDiff line numberDiff line change
@@ -337,7 +337,14 @@ export class ProcessPromise extends Promise<ProcessOutput> {
337337
...args: any[]
338338
): (Writable & PromiseLike<ProcessPromise & Writable>) | ProcessPromise {
339339
if (isStringLiteral(dest, ...args))
340-
return this.pipe($({ halt: true })(dest as TemplateStringsArray, ...args))
340+
return this.pipe(
341+
$({
342+
halt: true,
343+
ac: this._snapshot.ac,
344+
signal: this._snapshot.signal,
345+
})(dest as TemplateStringsArray, ...args)
346+
)
347+
341348
if (isString(dest))
342349
throw new Error('The pipe() method does not take strings. Forgot $?')
343350

test/cli.test.js

+25-8
Original file line numberDiff line numberDiff line change
@@ -15,13 +15,27 @@
1515
import assert from 'node:assert'
1616
import { test, describe, before, after } from 'node:test'
1717
import { fileURLToPath } from 'node:url'
18+
import net from 'node:net'
19+
import getPort from 'get-port'
1820
import '../build/globals.js'
1921
import { isMain, normalizeExt } from '../build/cli.js'
2022

2123
const __filename = fileURLToPath(import.meta.url)
2224
const spawn = $.spawn
2325
const nodeMajor = +process.versions?.node?.split('.')[0]
2426
const test22 = nodeMajor >= 22 ? test : test.skip
27+
const getServer = (resp = [], log = console.log) => {
28+
const server = net.createServer()
29+
server.on('connection', (conn) => {
30+
conn.on('data', (d) => {
31+
conn.write(resp.shift() || 'pong')
32+
})
33+
})
34+
server.stop = () => new Promise((resolve) => server.close(() => resolve()))
35+
server.start = (port) =>
36+
new Promise((resolve) => server.listen(port, () => resolve(server)))
37+
return server
38+
}
2539

2640
describe('cli', () => {
2741
// Helps detect unresolved ProcessPromise.
@@ -123,19 +137,22 @@ describe('cli', () => {
123137
assert.ok(p.stderr.endsWith(cwd + '\n'))
124138
})
125139

126-
test('scripts from https', async () => {
127-
const server = $`cat ${path.resolve('test/fixtures/echo.http')} | nc -l 8080`
140+
test('scripts from https 200', async () => {
141+
const resp = await fs.readFile(path.resolve('test/fixtures/echo.http'))
142+
const port = await getPort()
143+
const server = await getServer([resp]).start(port)
128144
const out =
129-
await $`node build/cli.js --verbose http://127.0.0.1:8080/echo.mjs`
145+
await $`node build/cli.js --verbose http://127.0.0.1:${port}/echo.mjs`
130146
assert.match(out.stderr, /test/)
131-
await server.kill()
147+
await server.stop()
132148
})
133149

134-
test('scripts from https not ok', async () => {
135-
const server = $`echo $'HTTP/1.1 500\n\n' | nc -l 8081`
136-
const out = await $`node build/cli.js http://127.0.0.1:8081`.nothrow()
150+
test('scripts from https 500', async () => {
151+
const port = await getPort()
152+
const server = await getServer(['HTTP/1.1 500\n\n']).listen(port)
153+
const out = await $`node build/cli.js http://127.0.0.1:${port}`.nothrow()
137154
assert.match(out.stderr, /Error: Can't get/)
138-
await server.kill()
155+
await server.stop()
139156
})
140157

141158
test('scripts with no extension', async () => {

test/core.test.js

+14
Original file line numberDiff line numberDiff line change
@@ -677,6 +677,20 @@ describe('core', () => {
677677
assert.match(message, /The operation was aborted/)
678678
}
679679
})
680+
681+
test('abort signal is transmittable through pipe', async () => {
682+
const ac = new AbortController()
683+
const { signal } = ac
684+
const p1 = $({ signal, nothrow: true })`echo test`
685+
const p2 = p1.pipe`sleep 999`
686+
setTimeout(() => ac.abort(), 50)
687+
688+
try {
689+
await p2
690+
} catch ({ message }) {
691+
assert.match(message, /The operation was aborted/)
692+
}
693+
})
680694
})
681695

682696
describe('kill()', () => {

0 commit comments

Comments
 (0)