Skip to content
Closed

ai slop #29106

Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
56 changes: 52 additions & 4 deletions src/runtime/webview/JSWebView.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -326,10 +326,38 @@ extern "C" size_t Bun__Chrome__autoDetect(char* out, size_t cap);
JSWebView* JSWebView::createChrome(JSGlobalObject* g, Structure* structure,
uint32_t width, uint32_t height, const WTF::String& userDataDir,
const WTF::String& path, const WTF::Vector<WTF::String>& extraArgv,
bool stdoutInherit, bool stderrInherit, const WTF::String& wsUrl, bool skipAutoDetect)
bool stdoutInherit, bool stderrInherit, const WTF::String& wsUrl, bool skipAutoDetect,
ChromeCreateFailure* outFailure)
{
auto* zig = defaultGlobalObject(g);
auto& t = CDP::transport();
auto setFailure = [&](ChromeCreateFailure f) {
if (outFailure) *outFailure = f;
};

// ensureSpawned routes through Bun__Chrome__ensure which has no
// Windows port (POSIX socketpair + --remote-debugging-pipe fd 3/4).
// The WebSocket connect path is fine on Windows — WebCore::WebSocket
// works, and Bun__Chrome__autoDetect reads DevToolsActivePort from
// %LOCALAPPDATA%. We only refuse when spawn is unavoidable.
//
// Call ensureSpawned unconditionally: it short-circuits with true
// when m_mode != None && !m_dead (singleton already alive — e.g.
// an earlier view connected via backend.url). Only translate an
// actual spawn failure into NotImplementedOnWindows — otherwise a
// second view with backend.url:false or backend.path on a process
// that's already got a live WebSocket transport would wrongly
// refuse even though no spawn is needed.
auto trySpawn = [&]() -> bool {
if (t.ensureSpawned(zig, userDataDir, path, extraArgv, stdoutInherit, stderrInherit))
return true;
#if OS(WINDOWS)
setFailure(ChromeCreateFailure::NotImplementedOnWindows);
#else
setFailure(ChromeCreateFailure::SpawnFailed);
#endif
Comment thread
coderabbitai[bot] marked this conversation as resolved.
return false;
};

// Transport selection, in priority order:
// 1. url: "ws://..." → connect (autoDetected=false → no fallback)
Expand All @@ -342,20 +370,40 @@ JSWebView* JSWebView::createChrome(JSGlobalObject* g, Structure* structure,
// sync/instant so the constructor stays synchronous.
bool ok;
if (!wsUrl.isEmpty()) {
// Explicit ws:// — pure WebSocket, no spawn path. Works on
// Windows too (WebCore::WebSocket is cross-platform).
ok = t.ensureConnected(zig, wsUrl, /* autoDetected */ false);
if (!ok) setFailure(ChromeCreateFailure::ConnectFailed);
} else if (skipAutoDetect || !path.isEmpty() || !extraArgv.isEmpty()) {
ok = t.ensureSpawned(zig, userDataDir, path, extraArgv, stdoutInherit, stderrInherit);
ok = trySpawn();
} else {
// Auto-detect. DevToolsActivePort URL caps at
// ws://127.0.0.1:65535/devtools/browser/<36-char-uuid> ≈ 70B.
// Bun__Chrome__autoDetect has a Windows branch (reads
// %LOCALAPPDATA%\Google\Chrome\User Data\DevToolsActivePort),
// so on Windows the connect-an-existing-Chrome flow works
// even though our own spawn path doesn't.
char buf[128];
size_t len = Bun__Chrome__autoDetect(buf, sizeof(buf));
if (len > 0) {
// autoDetected=true enables the wsOnClose stale-file
// fallback to ensureSpawned when the stored DevToolsActivePort
// was stale. On Windows there is no spawn path to fall back
// to, so pass false there — a stale file surfaces as a plain
// WebSocket connect failure.
#if OS(WINDOWS)
constexpr bool autoDetectedFallback = false;
#else
constexpr bool autoDetectedFallback = true;
#endif
ok = t.ensureConnected(zig,
WTF::String::fromUTF8(std::span<const char>(buf, len)),
/* autoDetected */ true, userDataDir, stdoutInherit, stderrInherit);
autoDetectedFallback, userDataDir, stdoutInherit, stderrInherit);
// AutoDetectConnectFailed (not ConnectFailed) — the user
// never set backend.url, so the error must not hint at it.
if (!ok) setFailure(ChromeCreateFailure::AutoDetectConnectFailed);
} else {
ok = t.ensureSpawned(zig, userDataDir, path, extraArgv, stdoutInherit, stderrInherit);
ok = trySpawn();
}
}
if (!ok) return nullptr;
Expand Down
28 changes: 27 additions & 1 deletion src/runtime/webview/JSWebView.h
Original file line number Diff line number Diff line change
Expand Up @@ -192,11 +192,37 @@ class JSWebView final : public WebCore::JSEventTarget {
// if Chrome spawn failed.
// wsUrl: connect to an existing Chrome's WebSocket debugger endpoint
// instead of spawning. Empty → spawn with --remote-debugging-pipe.
//
// ChromeCreateFailure distinguishes the nullptr reasons so the
// constructor can throw an accurate error. Default (SpawnFailed)
// is the old pre-Windows-fix behavior: "failed to spawn Chrome".
//
// ConnectFailed: sync failure constructing a WebSocket for a user-
// supplied `backend.url`. The URL was structurally bad enough for
// WebCore::WebSocket::create to throw (malformed scheme, etc.).
// Async handshake failures (stale port, Chrome down) flow through
// wsOnClose and surface as later promise rejections, not here.
//
// AutoDetectConnectFailed: same sync failure but for the URL we
// read from DevToolsActivePort ourselves — the user never set
// backend.url, so the error message must not hint at setting it.
// Only fires if the file contains a syntactically malformed URL
// (readDevToolsActivePort validates port/path but not the full
// ws:// form); normal stale-file cases flow through wsOnClose.
//
// NotImplementedOnWindows: the call would have needed the POSIX-only
// spawn path; `Bun__Chrome__ensure` has no Windows port.
enum class ChromeCreateFailure : uint8_t {
SpawnFailed = 0,
ConnectFailed,
AutoDetectConnectFailed,
NotImplementedOnWindows,
};
static JSWebView* createChrome(JSC::JSGlobalObject*, JSC::Structure*,
uint32_t width, uint32_t height, const WTF::String& userDataDir,
const WTF::String& path, const WTF::Vector<WTF::String>& extraArgv,
bool stdoutInherit, bool stderrInherit, const WTF::String& wsUrl = {},
bool skipAutoDetect = false);
bool skipAutoDetect = false, ChromeCreateFailure* outFailure = nullptr);

void finishCreation(JSC::VM&);
static JSC::Structure* createStructure(JSC::VM&, JSC::JSGlobalObject*, JSC::JSValue prototype);
Expand Down
49 changes: 43 additions & 6 deletions src/runtime/webview/JSWebViewConstructor.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -342,22 +342,59 @@ JSC_DEFINE_HOST_FUNCTION(constructWebView, (JSGlobalObject * globalObject, CallF

if (backend == WebViewBackend::Chrome) {
Bun__Feature__webview_chrome += 1;
JSWebView::ChromeCreateFailure failure = JSWebView::ChromeCreateFailure::SpawnFailed;
JSWebView* view = JSWebView::createChrome(globalObject, structure, width, height,
persistDir, chromePath, chromeArgv, stdoutInherit, stderrInherit, chromeWsUrl,
chromeSkipAutoDetect);
chromeSkipAutoDetect, &failure);
if (!view) {
return Bun::throwError(globalObject, scope, ErrorCode::ERR_DLOPEN_FAILED,
chromeWsUrl.isEmpty()
? "Failed to spawn Chrome (set BUN_CHROME_PATH, backend.path, or install Chrome/Chromium)"_s
: "Failed to connect to Chrome (check backend.url is a valid ws:// debugger endpoint)"_s);
// NotImplementedOnWindows is distinct from SpawnFailed: the
// POSIX socketpair + --remote-debugging-pipe fd plumbing in
// ChromeProcess.zig has no Windows port, so when the call
// actually needed the spawn path we surface it as a clean
// platform-status error. BUN_CHROME_PATH / backend.path are
// inert on that path, so the old "set the path" hint was
// actively misleading there (issue #29102). Explicit ws://
// connects and auto-detected existing Chrome still work on
// Windows — those paths never reach ensureSpawned.
switch (failure) {
case JSWebView::ChromeCreateFailure::NotImplementedOnWindows:
return Bun::throwError(globalObject, scope, ErrorCode::ERR_METHOD_NOT_IMPLEMENTED,
"Bun.WebView chrome backend spawn is not yet implemented on Windows; "
"connect to an already-running Chrome with backend: { type: \"chrome\", url: \"ws://...\" } instead"_s);
case JSWebView::ChromeCreateFailure::ConnectFailed:
return Bun::throwError(globalObject, scope, ErrorCode::ERR_DLOPEN_FAILED,
"Failed to connect to Chrome (check backend.url is a valid ws:// debugger endpoint)"_s);
Comment thread
robobun marked this conversation as resolved.
case JSWebView::ChromeCreateFailure::AutoDetectConnectFailed:
// Distinct from ConnectFailed: the user never set
// backend.url, so the error must not hint at it. The
// auto-detected URL came from DevToolsActivePort in
// Chrome's profile dir and was malformed enough for
// WebCore::WebSocket::create to fail synchronously.
return Bun::throwError(globalObject, scope, ErrorCode::ERR_DLOPEN_FAILED,
"Failed to connect to auto-detected Chrome (malformed DevToolsActivePort file)"_s);
case JSWebView::ChromeCreateFailure::SpawnFailed:
return Bun::throwError(globalObject, scope, ErrorCode::ERR_DLOPEN_FAILED,
"Failed to spawn Chrome (set BUN_CHROME_PATH, backend.path, or install Chrome/Chromium)"_s);
}
RELEASE_ASSERT_NOT_REACHED();
}
view->m_consoleIsGlobal = consoleIsGlobal;
if (consoleCallback) view->m_onConsole.set(vm, view, consoleCallback);
if (!initialUrl.isEmpty()) view->navigate(globalObject, initialUrl);
return JSValue::encode(view);
}

#if !OS(DARWIN)
#if OS(WINDOWS)
// Chrome spawn is unavailable on Windows (see the NotImplementedOnWindows
// branch above), so pointing users at `backend: "chrome"` as a fallback
// here would just lead them to a second ERR_METHOD_NOT_IMPLEMENTED. They
// can still connect to a running Chrome via backend.url, so tell them
// that instead.
return Bun::throwError(globalObject, scope, ErrorCode::ERR_METHOD_NOT_IMPLEMENTED,
"Bun.WebView with backend \"webkit\" is only available on macOS; "
"on Windows, connect to an already-running Chrome with "
"backend: { type: \"chrome\", url: \"ws://...\" }"_s);
#elif !OS(DARWIN)
return Bun::throwError(globalObject, scope, ErrorCode::ERR_METHOD_NOT_IMPLEMENTED,
"Bun.WebView with backend \"webkit\" is only available on macOS; use backend: \"chrome\""_s);
#else
Expand Down
90 changes: 87 additions & 3 deletions test/js/bun/webview/webview.test.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { dlopen, FFIType, ptr, toArrayBuffer } from "bun:ffi";
import { expect, test } from "bun:test";
import { bunEnv, bunExe, isCI, isMacOS, isMacOSVersionAtLeast, tempDir } from "harness";
import { bunEnv, bunExe, isCI, isMacOS, isMacOSVersionAtLeast, isWindows, tempDir } from "harness";

// FFI shm access for encoding:"shmem" tests. In real use Kitty (or
// whoever opens the segment) does this — shm_open + mmap + read + unlink.
Expand Down Expand Up @@ -61,17 +61,101 @@ const html = (h: string) => "data:text/html," + encodeURIComponent(h);
test("backend: 'webkit' throws on non-darwin", () => {
// Default backend is platform-dependent (WebKit on Darwin, Chrome
// elsewhere). Explicitly requesting WebKit off-Darwin should throw.
// On Windows the message differs (points at the ws:// connect
// workaround instead of "use backend: chrome" because chrome's
// spawn path is also not implemented there) — that's covered by
// the Windows-specific test below. The regex here is narrowed to
// require "use backend" so it doesn't incidentally match the
// Windows message's `backend: { type: "chrome", url: "ws://..." }`
// example text.
if (isMacOS) {
const view = new Bun.WebView({ width: 100, height: 100, backend: "webkit" });
expect(view).toBeInstanceOf(Bun.WebView);
view.close();
} else {
} else if (!isWindows) {
expect(() => new Bun.WebView({ width: 100, height: 100, backend: "webkit" })).toThrow(
/only available on macOS.*backend.*chrome/i,
/only available on macOS.*use backend.*chrome/i,
);
}
});

// https://github.com/oven-sh/bun/issues/29102 — Chrome backend's spawn
// path has no Windows implementation yet. Only test shapes that force
// spawn without consulting Bun__Chrome__autoDetect, so the test is
// deterministic regardless of whether Chrome is running on the host
// with --remote-debugging-port. Default `{}` and `backend: "chrome"`
// go through auto-detect and their spawn-path coverage lives in
// test/regression/issue/29102.test.ts where LOCALAPPDATA is scrubbed
// to guarantee the auto-detect branch misses.
test.skipIf(!isWindows)("backend: 'chrome' spawn throws on Windows", () => {
const cases: Array<object> = [
// Explicit path forces spawn-mode (skips auto-detect).
{
backend: {
type: "chrome",
path: "C:/Program Files/Google/Chrome/Application/chrome.exe",
},
},
// url:false also forces spawn-mode (documented knob).
{ backend: { type: "chrome", url: false } },
];
for (const opts of cases) {
let err: any;
let view: any;
try {
view = new (Bun as any).WebView(opts);
} catch (e) {
err = e;
}
if (view) {
// Unexpected success — close and fail loudly rather than leave
// a live view that could hang the suite.
try {
Comment thread
robobun marked this conversation as resolved.
view.close();
} catch {}
throw new Error(`UNEXPECTED_SUCCESS for opts=${JSON.stringify(opts)}: expected ERR_METHOD_NOT_IMPLEMENTED`);
}
Comment thread
robobun marked this conversation as resolved.
expect(err).toBeDefined();
expect(err.code).toBe("ERR_METHOD_NOT_IMPLEMENTED");
expect(err.message).toMatch(/chrome.*spawn.*not.*yet.*implemented.*windows/i);
// Positive: the message must point users at the ws:// connect
// workaround. Mirrors the pattern in 29102.test.ts's helpers —
// stripping the hint would otherwise pass silently.
expect(err.message).toMatch(/ws:\/\//i);
// Must not mention BUN_CHROME_PATH / set...backend.path — those
// knobs are inert on the Windows spawn path and the old message's
// hint at them is exactly what confused the bug reporter.
expect(err.message).not.toMatch(/BUN_CHROME_PATH/);
expect(err.message).not.toMatch(/set.*backend\.path/);
}
});

// Companion: `backend: 'webkit'` on Windows must not suggest "use
// backend: chrome" (which is now also spawn-gated on Windows), or the
// user would hit a second not-implemented error.
test.skipIf(!isWindows)("backend: 'webkit' on Windows does not point at a broken chrome fallback", () => {
let err: any;
let view: any;
try {
view = new (Bun as any).WebView({ width: 100, height: 100, backend: "webkit" });
} catch (e) {
err = e;
}
if (view) {
try {
view.close();
} catch {}
throw new Error("UNEXPECTED_SUCCESS: webkit should not work on Windows");
}
expect(err).toBeDefined();
expect(err.code).toBe("ERR_METHOD_NOT_IMPLEMENTED");
expect(err.message).toMatch(/only available on macOS/i);
// The bare "use backend: chrome" hint was misleading on Windows —
// chrome's spawn path is also not implemented. If we mention chrome
// at all, it must be as the ws:// connect workaround.
expect(err.message).toMatch(/ws:\/\//i);
});
Comment thread
robobun marked this conversation as resolved.

test("calling without new throws", () => {
expect(() => (Bun.WebView as any)({ width: 100, height: 100 })).toThrow(/without 'new'/);
});
Expand Down
Loading
Loading