starred/windmill
1
0
Fork 0
mirror of https://github.com/windmill-labs/windmill.git synced 2026-09-04 16:03:06 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(debugger): parse bun 1.4's UUID inspector token (#10828)

Browse Source 
* fix(debugger): parse bun 1.4's UUID inspector token

Bun 1.4 changed the inspector URL's token to a hyphenated UUID. The stderr
scraper matched `[a-z0-9]+`, so it stopped at the first hyphen and connected to
a truncated path, which the inspector answers with 404. Every TypeScript debug
session has failed to attach since the 1.4.0 bump, taking the windmill-extra
integration tests with it.

Match the whole path, and only once its line is newline-terminated: a stderr
chunk can end mid-URL and would otherwise be read as a complete, truncated URL.

A close before the handshake completes is now reported as the connection
failure it is, rather than as a finished script, and the debuggee is reaped -
--inspect-wait blocks until a debugger attaches, so a failed attach leaked a bun
process per session.

On the test client, queue events that arrive before their waiter registers: the
server sends 'initialized' immediately behind the 'initialize' response, which
the client could drop and then time out waiting for.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XMizaQRcnWRd79t5wWhjBN

* fix(debugger): keep the first terminated event's result on launch failure

A socket that drops after the handshake opens but mid-command-sequence reports
the termination from onclose, carrying the script result, and then fails the
launch. Sending a second terminated from the failure path overwrote that result
with an error-only event. Guard the send the way every other emit site in the
file does, leaving the reaping unconditional.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XMizaQRcnWRd79t5wWhjBN

* fix(debugger): report an inspector drop during setup as the failure it is

The setup commands run over an open socket and none of them reject when it
drops - sendInspectorCommand only has its own timer - so a drop between the
upgrade and Inspector.initialized was reported as a clean termination, and the
error surfaced up to 10s later or, once the duplicate was guarded, not at all.
Draw the line at execution actually starting rather than at the socket opening,
so those failures terminate with the connection error, immediately and once.

Pair the "Failed to start Bun" output with the terminated event it explains,
so a run that already reported its result cannot also be told it failed to
launch.

Prove the inspector URL complete with whitespace rather than an end-of-line:
trailing text on the banner line would otherwise stall the parse for 10s.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XMizaQRcnWRd79t5wWhjBN

* fix(debugger): mark execution started only once the start command is answered

Inspector.initialized is what starts the script, so setting the flag before
awaiting its reply left a drop during that round trip looking like a clean
termination - the same silent failure, narrowed to one command. Its reply
precedes any close on the socket, so the continuation still runs before onclose
and a real run is not misread as a failure.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01XMizaQRcnWRd79t5wWhjBN

---------

Co-authored-by: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
Ruben Fiszel
2026-08-26 00:38:09 +02:00
committed by GitHub
parent 5dc43c400a
commit 4658224592
2 changed files with 69 additions and 16 deletions
Download Patch File Download Diff File Expand all files Collapse all files
debugger/dap_websocket_server_bun.ts
+45 -5
View File
@@ -1651,8 +1651,17 @@ export class DebugSession {
try {
await this.startBunProcess(cwd)
} catch (error) {
this.sendEvent('output', { category: 'stderr', output: `Failed to start Bun: ${error}\n` })
this.sendEvent('terminated', { error: String(error) })
// A launch failure is reported here, a finished script from onclose; whichever gets
// there first owns the terminated event, so a client that already has a result is
// never told afterwards that the launch failed.
if (!this.terminatedSent) {
this.terminatedSent = true
this.sendEvent('output', { category: 'stderr', output: `Failed to start Bun: ${error}\n` })
this.sendEvent('terminated', { error: String(error) })
}
// --inspect-wait blocks until a debugger attaches, so a bun we failed to attach to
// waits forever unless it is reaped here.
await this.cleanup()
}
}
@@ -1956,9 +1965,12 @@ export class DebugSession {
const text = decoder.decode(value)
buffer += text
// Look for the WebSocket URL in Bun's inspector output
// Format: "ws://127.0.0.1:9229/xxxxx"
const wsMatch = buffer.match(/ws:\/\/[\d.]+:\d+\/[a-z0-9]+/i)
// Look for the WebSocket URL in Bun's inspector banner, e.g.
// " ws://127.0.0.1:9229/848c719d-a52e-4610-8e94-99cd60f34af9".
// The token's alphabet is Bun's to change (it became a hyphenated UUID in 1.4), so
// take the whole path, and only once whitespace proves it complete: a stderr chunk
// can end mid-URL, and connecting to a truncated path gets a 404 from the inspector.
const wsMatch = buffer.match(/ws:\/\/[\d.]+:\d+\/\S+(?=\s)/)
if (wsMatch && this.inspectorWsUrlPromise) {
const wsUrl = wsMatch[0]
logger.info(`Found inspector WebSocket URL in stderr: ${wsUrl}`)
@@ -1988,12 +2000,22 @@ export class DebugSession {
return new Promise((resolve, reject) => {
this.inspectorWs = new WebSocket(wsUrl)
// A close before the script is running is a failed connection, not a finished script,
// and the two are reported to the client in opposite ways. The socket opening is not
// the line: the setup commands below run over an open socket and none of them reject
// when it drops (sendInspectorCommand only has its own timer), so a drop mid-setup
// would otherwise be indistinguishable from a clean exit.
let opened = false
let executionStarted = false
let handshakeError: string | null = null
const timeout = setTimeout(() => {
reject(new Error('Inspector connection timeout'))
}, 5000)
this.inspectorWs.onopen = async () => {
clearTimeout(timeout)
opened = true
logger.info('Connected to inspector')
try {
@@ -2030,6 +2052,11 @@ export class DebugSession {
logger.info('Starting script execution with Inspector.initialized...')
await this.sendInspectorCommand('Inspector.initialized', {})
// Only past its reply is a later close a finished script rather than a lost
// connection. The reply precedes any close on this socket, so the continuation
// runs first and a real run is never misread as a failure.
executionStarted = true
resolve()
} catch (error) {
reject(error)
@@ -2042,12 +2069,25 @@ export class DebugSession {
this.inspectorWs.onerror = (error) => {
logger.error('Inspector WebSocket error:', error)
if (!opened) {
handshakeError = (error as ErrorEvent)?.message || String(error)
}
}
this.inspectorWs.onclose = () => {
logger.info('Inspector WebSocket closed')
this.inspectorWs = null
if (!executionStarted) {
clearTimeout(timeout)
reject(
new Error(
`Inspector connection failed: ${handshakeError ?? (opened ? 'closed before setup completed' : 'closed before the handshake completed')}`
)
)
return
}
// When inspector closes, the script has ended - send terminated event
if (!this.terminatedSent) {
this.terminatedSent = true
docker/test_windmill_extra.ts
+24 -11
View File
@@ -206,7 +206,11 @@ class DAPTestClient {
private events: DAPMessage[] = []
private output: string[] = []
private result: unknown = undefined
private eventHandlers = new Map<string, ((event: DAPMessage) => void)[]>()
private eventWaiters = new Map<string, ((event: DAPMessage) => void)[]>()
// An event that arrives before its waiter is registered is queued rather than dropped: the
// server sends 'initialized' right behind the 'initialize' response, and 'terminated' can
// land before the launch call the test awaits has even returned.
private bufferedEvents = new Map<string, DAPMessage[]>()
async connect(endpoint: string): Promise<void> {
const url = `ws://${HOST}:${DEBUGGER_PORT}${endpoint}`
@@ -273,9 +277,13 @@ class DAPTestClient {
this.result = msg.body.result
}
const handlers = this.eventHandlers.get(msg.event!) || []
for (const handler of handlers) {
handler(msg)
const waiters = this.eventWaiters.get(msg.event!)
if (waiters && waiters.length > 0) {
waiters.shift()!(msg)
} else {
const buffered = this.bufferedEvents.get(msg.event!) || []
buffered.push(msg)
this.bufferedEvents.set(msg.event!, buffered)
}
}
} catch {
@@ -313,23 +321,27 @@ class DAPTestClient {
}
waitForEvent(eventName: string, timeout = 10000): Promise<DAPMessage> {
const buffered = this.bufferedEvents.get(eventName)
if (buffered && buffered.length > 0) {
return Promise.resolve(buffered.shift()!)
}
return new Promise((resolve, reject) => {
const waiters = this.eventWaiters.get(eventName) || []
this.eventWaiters.set(eventName, waiters)
const timer = setTimeout(() => {
const idx = waiters.indexOf(handler)
if (idx >= 0) waiters.splice(idx, 1)
reject(new Error(`Timeout waiting for event: ${eventName}`))
}, timeout)
const handler = (event: DAPMessage) => {
clearTimeout(timer)
const handlers = this.eventHandlers.get(eventName) || []
const idx = handlers.indexOf(handler)
if (idx >= 0) handlers.splice(idx, 1)
resolve(event)
}
if (!this.eventHandlers.has(eventName)) {
this.eventHandlers.set(eventName, [])
}
this.eventHandlers.get(eventName)!.push(handler)
waiters.push(handler)
})
}
@@ -379,6 +391,7 @@ class DAPTestClient {
this.output = []
this.result = undefined
this.events = []
this.bufferedEvents.clear()
}
}