Skip to content
Merged
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
71 changes: 71 additions & 0 deletions apps/desktop/electron/connection-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,7 @@ import {
profileRemoteOverride,
profileSshOverride,
resolveAuthMode,
resolveProfileBackendRoute,
resolveTestWsUrl,
RT_COOKIE_VARIANTS,
savedProfileSsh,
Expand Down Expand Up @@ -187,6 +188,65 @@ test('saved SSH drafts are inactive and explicit overrides take precedence', ()
assert.equal(profileHasRemoteConnection(config, 'coder'), true)
})

// --- resolveProfileBackendRoute ---

const ROUTES = [
{
name: 'the primary profile owns the window backend',
profile: 'default',
opts: { primaryProfile: 'default' },
expected: { backend: 'primary', descriptorProfile: null, scopePath: false }
},
{
name: 'a renamed primary profile still owns the window backend',
profile: ' coder ',
opts: { primaryProfile: 'coder', globalRemote: true },
expected: { backend: 'primary', descriptorProfile: null, scopePath: false }
},
{
name: 'an unset profile resolves to the primary',
profile: '',
opts: { primaryProfile: 'default', globalRemote: true },
expected: { backend: 'primary', descriptorProfile: null, scopePath: false }
},
{
name: 'a profile inheriting the app-global remote shares the primary backend, scoped per request',
profile: 'coder',
opts: { primaryProfile: 'default', globalRemote: true, profileRemoteOverride: false },
expected: { backend: 'primary', descriptorProfile: 'coder', scopePath: true }
},
{
name: 'a profile with its own remote override gets a pooled descriptor for that host',
profile: 'coder',
opts: { primaryProfile: 'default', globalRemote: true, profileRemoteOverride: true },
expected: { backend: 'pool', descriptorProfile: null, scopePath: false }
},
{
name: 'a local non-primary profile gets its own pooled backend',
profile: 'coder',
opts: { primaryProfile: 'default', globalRemote: false, profileRemoteOverride: false },
expected: { backend: 'pool', descriptorProfile: null, scopePath: false }
}
]

for (const route of ROUTES) {
test(`resolveProfileBackendRoute: ${route.name}`, () => {
assert.deepEqual(resolveProfileBackendRoute(route.profile, route.opts), route.expected)
})
}

test('resolveProfileBackendRoute only tags a descriptor when the backend is shared', () => {
// A pooled backend is already scoped to its profile, so tagging it would
// imply a second scope the caller must reconcile. Only the shared
// global-remote route carries one.
for (const route of ROUTES) {
const resolved = resolveProfileBackendRoute(route.profile, route.opts)

assert.equal(Boolean(resolved.descriptorProfile), resolved.scopePath)
assert.ok(!resolved.descriptorProfile || resolved.backend === 'primary')
}
})

// --- pathWithGlobalRemoteProfile ---

test('pathWithGlobalRemoteProfile appends profile in global remote mode', () => {
Expand All @@ -199,6 +259,17 @@ test('pathWithGlobalRemoteProfile appends profile in global remote mode', () =>
)
})

test('pathWithGlobalRemoteProfile skips the primary profile, which the remote already serves', () => {
assert.equal(
pathWithGlobalRemoteProfile('/api/model/info', 'coder', {
globalRemote: true,
primaryProfile: 'coder',
profileRemoteOverride: false
}),
'/api/model/info'
)
})

test('pathWithGlobalRemoteProfile preserves existing query params', () => {
assert.equal(
pathWithGlobalRemoteProfile('/api/model/options?force=1', 'iris', {
Expand Down
65 changes: 59 additions & 6 deletions apps/desktop/electron/connection-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -350,16 +350,68 @@ function profileRemoteOverride(config, profile) {
return { url, authMode: normAuthMode(entry.authMode), token: entry.token }
}

export interface ProfileRouteOptions {
globalRemote?: boolean
primaryProfile?: null | string
profileRemoteOverride?: boolean
}

export interface ProfileBackendRoute {
/** Which backend serves this profile: the window backend, or a pooled one. */
backend: 'pool' | 'primary'
/**
* Profile to tag on the returned descriptor when the backend is shared and
* therefore not itself scoped to that profile. Null when the backend already
* belongs to the profile.
*/
descriptorProfile: null | string
/** Whether REST paths on this route must carry `?profile=` to be scoped. */
scopePath: boolean
}

/**
* The one place that answers "which backend serves profile P, and does its
* REST path need a profile scope?". Four routes, in precedence order:
*
* 1. The primary profile owns the window backend outright.
* 2. A profile with its own remote override gets a pooled descriptor for that
* host, which is already scoped to it.
* 3. A profile inheriting the app-global remote shares the primary backend —
* one host serves every profile — so it is scoped per request instead.
* 4. Any other local profile gets its own pooled backend, spawned with
* `--profile`, so its `HERMES_HOME` scopes it.
*
* Routing used to be spread across three overlapping predicates that each
* re-derived part of this table, which is how case 3 ended up registering
* reapable pool entries for backends it never owned.
*/
function resolveProfileBackendRoute(profile, opts: ProfileRouteOptions = {}): ProfileBackendRoute {
const scopedProfile = connectionScopeKey(profile)
const primaryProfile = connectionScopeKey(opts.primaryProfile) || 'default'

if (!scopedProfile || scopedProfile === primaryProfile) {
return { backend: 'primary', descriptorProfile: null, scopePath: false }
}

if (opts.profileRemoteOverride) {
return { backend: 'pool', descriptorProfile: null, scopePath: false }
}

if (opts.globalRemote) {
return { backend: 'primary', descriptorProfile: scopedProfile, scopePath: true }
}

return { backend: 'pool', descriptorProfile: null, scopePath: false }
}

/**
* In global-remote mode one backend serves every Desktop profile, so REST calls
* that are scoped by renderer-side `request.profile` must carry that scope as a
* query parameter. Local pooled backends and per-profile remote overrides do not
* need this: they already run against a backend scoped to the target profile.
* Add renderer-side `request.profile` to a REST path when the route says the
* serving backend is not already scoped to that profile.
*/
function pathWithGlobalRemoteProfile(path, profile, opts: any = {}) {
function pathWithGlobalRemoteProfile(path, profile, opts: ProfileRouteOptions = {}) {
const scopedProfile = connectionScopeKey(profile)

if (!scopedProfile || !opts.globalRemote || opts.profileRemoteOverride) {
if (!resolveProfileBackendRoute(profile, opts).scopePath) {
return path
}

Expand Down Expand Up @@ -506,6 +558,7 @@ export {
profileRemoteOverride,
profileSshOverride,
resolveAuthMode,
resolveProfileBackendRoute,
resolveTestWsUrl,
RT_COOKIE_VARIANTS,
savedProfileSsh,
Expand Down
70 changes: 70 additions & 0 deletions apps/desktop/electron/crash-forensics.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
import { describe, expect, it, vi } from 'vitest'

import { describeCrashReason, installCrashForensics } from './crash-forensics'

const harness = () => {
const listeners = new Map<string, (value: unknown) => void>()
const flush = vi.fn()
const log = vi.fn()

installCrashForensics({
flush,
log,
target: { on: (event, listener) => listeners.set(event, listener) }
})

return { flush, listeners, log }
}

describe('describeCrashReason', () => {
it('prefers a stack, then a message, for thrown errors', () => {
const withStack = new Error('boom')
withStack.stack = 'Error: boom\n at somewhere'

expect(describeCrashReason(withStack)).toBe('Error: boom\n at somewhere')

const withoutStack = new Error('boom')
withoutStack.stack = ''

expect(describeCrashReason(withoutStack)).toBe('boom')
})

it('renders non-error rejections without throwing', () => {
expect(describeCrashReason('plain string')).toBe('plain string')
expect(describeCrashReason({ code: 'ECONNRESET' })).toBe('{"code":"ECONNRESET"}')
expect(describeCrashReason(undefined)).toBe('undefined')

const circular: Record<string, unknown> = {}
circular.self = circular

expect(describeCrashReason(circular)).toBe('[object Object]')
})
})

describe('installCrashForensics', () => {
it('records and synchronously flushes an uncaught exception', () => {
const { flush, listeners, log } = harness()
const error = new Error('renderer gone')
error.stack = 'Error: renderer gone\n at main'

listeners.get('uncaughtException')?.(error)

expect(log).toHaveBeenCalledWith('[main] Uncaught exception: Error: renderer gone\n at main')
expect(flush).toHaveBeenCalledTimes(1)
})

it('records and synchronously flushes an unhandled rejection', () => {
const { flush, listeners, log } = harness()

listeners.get('unhandledRejection')?.('gateway ticket mint failed')

expect(log).toHaveBeenCalledWith('[main] Unhandled rejection: gateway ticket mint failed')
expect(flush).toHaveBeenCalledTimes(1)
})

it('registers both handlers', () => {
const { listeners } = harness()

expect([...listeners.keys()].sort()).toEqual(['uncaughtException', 'unhandledRejection'])
})
})
51 changes: 51 additions & 0 deletions apps/desktop/electron/crash-forensics.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,51 @@
/**
* Last-chance forensics for the Electron main process.
*
* Electron installs its own `uncaughtException` listener and only warns on
* unhandled rejections, so the app usually survives — but the reason lands on
* stderr alone, which is discarded entirely when the app is launched from
* Finder or the Start menu. Without a record in desktop.log, a main-process
* fault is invisible in a `hermes debug share` bundle and the user is left
* describing symptoms instead of showing a stack.
*/

export interface CrashForensicsTarget {
on: (event: 'uncaughtException' | 'unhandledRejection', listener: (value: unknown) => void) => unknown
}

export interface CrashForensicsOptions {
flush: () => void
log: (message: string) => void
target?: CrashForensicsTarget
}

/** Render a thrown value for the log, preferring a stack over a bare message. */
export function describeCrashReason(reason: unknown): string {
if (reason instanceof Error) {
return reason.stack || reason.message || reason.name || 'Error'
}

if (typeof reason === 'string') {
return reason
}

try {
return JSON.stringify(reason) ?? String(reason)
} catch {
return String(reason)
}
}

/**
* Record main-process faults to desktop.log and flush synchronously, since a
* fault that does prove fatal leaves no chance for the batched async flush.
*/
export function installCrashForensics({ flush, log, target = process }: CrashForensicsOptions): void {
const record = (label: string) => (reason: unknown) => {
log(`[main] ${label}: ${describeCrashReason(reason)}`)
flush()
}

target.on('uncaughtException', record('Uncaught exception'))
target.on('unhandledRejection', record('Unhandled rejection'))
}
Loading
Loading