Skip to content

Commit 76d1ec3

Browse files
DavertMikclaude
andcommitted
fix(CDPBrowser): address review on Obscura binary lookup and Kitesurf session release
Obscura._resolveBinary no longer shells out to `which`, which does not exist on Windows, so an obscura.exe on PATH was never discovered. It now walks PATH directly and tries every PATHEXT suffix on Windows, strips the quotes Windows allows around PATH entries, and skips directories and non-executable files. Kitesurf._finishTest releases the Cloudflare session in a finally block. If super._finishTest() rejected while closing the CDP connection the DELETE was skipped and the browser session stayed alive until keepAlive expired. The session id is cleared before the request so a repeated call cannot release it twice. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
1 parent 3b2c044 commit 76d1ec3

2 files changed

Lines changed: 35 additions & 13 deletions

File tree

lib/helper/Kitesurf.js

Lines changed: 12 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -116,16 +116,22 @@ class Kitesurf extends CDPBrowser {
116116
/**
117117
* Closes the target as `CDPBrowser._finishTest` does, then releases the cloud session acquired
118118
* in `_resolveEndpoint` via the Cloudflare API so it does not linger for the full `keepAlive`
119-
* window.
119+
* window. The release runs in a `finally` so a rejection while closing the CDP connection still
120+
* frees the cloud session instead of leaving the browser alive until `keepAlive` expires; the
121+
* session id is cleared before the request, so a repeated call never releases it twice.
120122
*
121123
* @protected
122124
*/
123125
async _finishTest() {
124-
await super._finishTest()
125-
if (this.cloudSessionId) {
126-
const url = `${this.options.apiBase}/accounts/${this.options.accountId}/browser-run/devtools/browser/${this.cloudSessionId}`
127-
await axios.delete(url, { headers: { Authorization: `Bearer ${this.options.apiToken}` } }).catch(() => null)
128-
this.cloudSessionId = null
126+
try {
127+
await super._finishTest()
128+
} finally {
129+
if (this.cloudSessionId) {
130+
const sessionId = this.cloudSessionId
131+
this.cloudSessionId = null
132+
const url = `${this.options.apiBase}/accounts/${this.options.accountId}/browser-run/devtools/browser/${sessionId}`
133+
await axios.delete(url, { headers: { Authorization: `Bearer ${this.options.apiToken}` } }).catch(() => null)
134+
}
129135
}
130136
}
131137
}

lib/helper/Obscura.js

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,10 @@
1-
import { spawn, execSync } from 'child_process'
1+
import { spawn } from 'child_process'
2+
import fs from 'fs'
23
import net from 'net'
4+
import path from 'path'
35
import axios from 'axios'
46
import CDPBrowser from './CDPBrowser.js'
7+
import { isFile, isWindows } from '../utils.js'
58

69
/**
710
* ## Configuration
@@ -201,20 +204,33 @@ class Obscura extends CDPBrowser {
201204

202205
/**
203206
* Resolves the `obscura` binary to spawn, in priority order: `options.binaryPath`, then the
204-
* `OBSCURA_PATH` environment variable, then `obscura` on `PATH` (via `which`).
207+
* `OBSCURA_PATH` environment variable, then `obscura` on `PATH`. The `PATH` lookup walks the
208+
* directories itself instead of shelling out to `which`, which does not exist on Windows: on
209+
* Windows every `PATHEXT` suffix is tried, so an `obscura.exe` on `PATH` is found too.
205210
*
206211
* @returns {string|null} an absolute or relative path to the binary, or null if none resolved.
207212
* @protected
208213
*/
209214
_resolveBinary() {
210215
if (this.options.binaryPath) return this.options.binaryPath
211216
if (process.env.OBSCURA_PATH) return process.env.OBSCURA_PATH
212-
try {
213-
const found = execSync('which obscura', { stdio: ['ignore', 'pipe', 'ignore'] }).toString().trim()
214-
return found || null
215-
} catch (e) {
216-
return null
217+
const windows = isWindows()
218+
const extensions = windows ? (process.env.PATHEXT || '.EXE;.CMD;.BAT;.COM').split(';') : ['']
219+
for (const entry of (process.env.PATH || '').split(path.delimiter)) {
220+
const dir = windows ? entry.replace(/^"|"$/g, '') : entry
221+
if (!dir) continue
222+
for (const extension of extensions) {
223+
const candidate = path.join(dir, `obscura${extension}`)
224+
if (!isFile(candidate)) continue
225+
try {
226+
fs.accessSync(candidate, fs.constants.X_OK)
227+
return candidate
228+
} catch (e) {
229+
continue
230+
}
231+
}
217232
}
233+
return null
218234
}
219235

220236
/**

0 commit comments

Comments
 (0)