Skip to content

Commit 08dab97

Browse files
committed
fix: release keep-alive sockets when closing themes:preview server
Make the command's close() async and call server.closeAllConnections() (Node 18.2+) before server.close(). Without this the undici fetch pool keeps sockets alive on Linux, preventing server.close() from resolving and leaving the Node event loop with live handles — which hung mocha on ubuntu CI since .mocharc.json has no --exit flag. Also makes Ctrl-C teardown reliable for users. Also makes the serialize-uploads preview test deterministic by gating the stubbed upload on a test-controlled resolver rather than asserting upload counts against real chokidar fs events.
1 parent 12a176c commit 08dab97

2 files changed

Lines changed: 34 additions & 13 deletions

File tree

packages/zcli-themes/src/commands/themes/preview.ts

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -132,11 +132,17 @@ export default class Preview extends Command {
132132
.on('unlink', handleThemeChange)
133133

134134
return {
135-
close: () => {
136-
// Stop watching file changes before terminating the server
137-
watcher.close()
138-
server.close()
135+
close: async () => {
136+
await watcher.close()
139137
wss.close()
138+
// Destroy keep-alive sockets so server.close() doesn't wait on undici's
139+
// idle timeout. closeAllConnections requires Node 18.2+; @types/node
140+
// v14 doesn't know about it yet.
141+
const serverWithCloseAll = server as typeof server & { closeAllConnections?: () => void }
142+
serverWithCloseAll.closeAllConnections?.()
143+
await new Promise<void>((resolve, reject) =>
144+
server.close(err => err ? reject(err) : resolve())
145+
)
140146
}
141147
}
142148
}

packages/zcli-themes/tests/functional/preview.test.ts

Lines changed: 24 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ describe('themes:preview', function () {
2121
})
2222

2323
describe('successful preview', () => {
24-
let server: { close: () => void }
24+
let server: { close: () => Promise<void> }
2525

2626
const preview = test
2727
.stdout()
@@ -40,8 +40,8 @@ describe('themes:preview', function () {
4040
server = await PreviewCommand.run([baseThemePath, '--bind', '0.0.0.0', '--port', '9999'])
4141
})
4242

43-
afterEach(() => {
44-
server.close()
43+
afterEach(async () => {
44+
await server.close()
4545
})
4646

4747
preview
@@ -134,20 +134,35 @@ describe('themes:preview', function () {
134134
preview
135135
.it('should serialize rapid successive changes instead of uploading concurrently', async () => {
136136
await waitForWatcherReady()
137+
// Gate each upload on a test-controlled resolver so assertions don't
138+
// depend on how the OS's chokidar backend coalesces fs events.
139+
const pendingResolvers: Array<() => void> = []
140+
fetchStub.withArgs(sinon.match({
141+
url: 'https://z3ntest.zendesk.com/hc/api/internal/theming/local_preview',
142+
method: 'PUT'
143+
})).callsFake(() => new Promise(resolve => {
144+
pendingResolvers.push(() => resolve({
145+
status: 200,
146+
ok: true,
147+
text: () => Promise.resolve('')
148+
}))
149+
}))
137150
const initialCalls = previewUploadCalls()
138151
const partialsDir = path.join(baseThemePath, 'templates/partials')
139152
fs.mkdirSync(partialsDir, { recursive: true })
140153
const paths = ['a.hbs', 'b.hbs', 'c.hbs'].map(name => path.join(partialsDir, name))
141154
try {
142155
for (const p of paths) fs.writeFileSync(p, '<div/>')
143156
await waitForUploadCount(initialCalls + 1)
144-
// Let any queued rerun finish; a working serializer produces at
145-
// most one in-flight upload plus one queued rerun.
146-
await new Promise(resolve => setTimeout(resolve, 500))
147-
const calls = previewUploadCalls() - initialCalls
148-
expect(calls).to.be.greaterThan(0)
149-
expect(calls).to.be.at.most(2)
157+
expect(previewUploadCalls() - initialCalls).to.eq(1)
158+
pendingResolvers.shift()?.()
159+
await waitForUploadCount(initialCalls + 2)
160+
pendingResolvers.shift()?.()
161+
await new Promise(resolve => setTimeout(resolve, 100))
162+
expect(previewUploadCalls() - initialCalls).to.eq(2)
150163
} finally {
164+
// Drain pending uploads so server.close() in afterEach isn't blocked.
165+
pendingResolvers.forEach(fn => fn())
151166
fs.rmSync(partialsDir, { recursive: true, force: true })
152167
}
153168
})

0 commit comments

Comments
 (0)