fix(desktop): don't fall back to a dead WS ticket on OAuth re-mint failure
The reconnect and boot paths resolved the WS URL with `(await getGatewayWsUrl().catch(() => null)) || conn.wsUrl`. For OAuth gateways the cached conn.wsUrl carries a single-use, ~30s-TTL ticket; the desktop connection is memoized for the process lifetime, so on reconnect that ticket is both expired and already consumed. A failed fresh mint therefore fell back to a guaranteed-dead ticket and surfaced as an opaque "connection closed", masking the gateway's actionable "session expired, sign in again" message. Extract resolveGatewayWsUrl() (with unit tests): in OAuth mode a mint failure throws a tagged GatewayReauthRequiredError instead of falling back; token/local modes keep the long-lived-token fallback. Thread that error through the reconnect path so requestGateway surfaces the reauth message rather than the generic transport error that triggered the retry. Co-authored-by: Kenmege <205099287+Kenmege@users.noreply.github.com>
This commit is contained in:
@ -2,6 +2,7 @@ import { useEffect, useRef } from 'react'
|
||||
|
||||
import type { HermesConnection } from '@/global'
|
||||
import { HermesGateway } from '@/hermes'
|
||||
import { resolveGatewayWsUrl } from '@/lib/gateway-ws-url'
|
||||
import {
|
||||
$desktopBoot,
|
||||
applyDesktopBootProgress,
|
||||
@ -232,8 +233,10 @@ export function useGatewayBoot({
|
||||
publish(conn)
|
||||
// Mint a fresh WS URL right before connecting. For OAuth gateways the
|
||||
// ticket is single-use with a short TTL, so the ticket baked into
|
||||
// conn.wsUrl can already be stale; getGatewayWsUrl() re-mints it.
|
||||
const wsUrl = (await desktop.getGatewayWsUrl?.().catch(() => null)) || conn.wsUrl
|
||||
// conn.wsUrl is stale; resolveGatewayWsUrl() re-mints it and, on
|
||||
// failure, throws a reauth error rather than connecting with a dead
|
||||
// ticket (which would surface as an opaque "connection closed").
|
||||
const wsUrl = await resolveGatewayWsUrl(desktop, conn)
|
||||
await gateway.connect(wsUrl)
|
||||
|
||||
if (cancelled) {
|
||||
|
||||
@ -2,6 +2,7 @@ import { useStore } from '@nanostores/react'
|
||||
import { useCallback, useEffect, useRef } from 'react'
|
||||
|
||||
import type { HermesGateway } from '@/hermes'
|
||||
import { isGatewayReauthRequired, resolveGatewayWsUrl } from '@/lib/gateway-ws-url'
|
||||
import { $gatewayState, setConnection } from '@/store/session'
|
||||
|
||||
export function useGatewayRequest() {
|
||||
@ -14,6 +15,10 @@ export function useGatewayRequest() {
|
||||
|
||||
const gatewayStateRef = useRef(gatewayState)
|
||||
const reconnectingRef = useRef<Promise<HermesGateway | null> | null>(null)
|
||||
// Holds the reauth error from the most recent failed reconnect so
|
||||
// requestGateway can surface the gateway's "session expired, sign in again"
|
||||
// message instead of the opaque "connection closed" that triggered the retry.
|
||||
const reauthErrorRef = useRef<unknown>(null)
|
||||
|
||||
useEffect(() => {
|
||||
gatewayStateRef.current = gatewayState
|
||||
@ -41,17 +46,26 @@ export function useGatewayRequest() {
|
||||
return null
|
||||
}
|
||||
|
||||
reauthErrorRef.current = null
|
||||
|
||||
try {
|
||||
const conn = await desktop.getConnection()
|
||||
connectionRef.current = conn
|
||||
setConnection(conn)
|
||||
// Re-mint the WS URL before reconnecting — OAuth tickets are single-use
|
||||
// and short-lived, so the cached conn.wsUrl ticket is stale here.
|
||||
const wsUrl = (await desktop.getGatewayWsUrl?.().catch(() => null)) || conn.wsUrl
|
||||
// Re-mint the WS URL before reconnecting. OAuth tickets are single-use
|
||||
// and short-lived, so the cached conn.wsUrl ticket is dead here;
|
||||
// resolveGatewayWsUrl() throws a reauth error in OAuth mode rather than
|
||||
// connecting with a stale ticket. Stash it so requestGateway can show
|
||||
// the actionable "sign in again" message.
|
||||
const wsUrl = await resolveGatewayWsUrl(desktop, conn)
|
||||
await existing.connect(wsUrl)
|
||||
|
||||
return existing
|
||||
} catch {
|
||||
} catch (error) {
|
||||
if (isGatewayReauthRequired(error)) {
|
||||
reauthErrorRef.current = error
|
||||
}
|
||||
|
||||
connectionRef.current = null
|
||||
setConnection(null)
|
||||
|
||||
@ -84,6 +98,15 @@ export function useGatewayRequest() {
|
||||
const recovered = await ensureGatewayOpen()
|
||||
|
||||
if (!recovered) {
|
||||
// Prefer the reauth error from the failed reconnect (OAuth session
|
||||
// expired) over the generic transport error that triggered the retry.
|
||||
const reauthError = reauthErrorRef.current
|
||||
reauthErrorRef.current = null
|
||||
|
||||
if (reauthError) {
|
||||
throw reauthError
|
||||
}
|
||||
|
||||
throw error
|
||||
}
|
||||
|
||||
|
||||
78
apps/desktop/src/lib/gateway-ws-url.test.ts
Normal file
78
apps/desktop/src/lib/gateway-ws-url.test.ts
Normal file
@ -0,0 +1,78 @@
|
||||
import { describe, expect, it, vi } from 'vitest'
|
||||
|
||||
import { GatewayReauthRequiredError, isGatewayReauthRequired, resolveGatewayWsUrl } from './gateway-ws-url'
|
||||
|
||||
const oauthConn = { authMode: 'oauth' as const, wsUrl: 'ws://host/api/ws?ticket=stale' }
|
||||
const tokenConn = { authMode: 'token' as const, wsUrl: 'ws://host/api/ws?token=abc' }
|
||||
|
||||
describe('resolveGatewayWsUrl', () => {
|
||||
describe('oauth mode', () => {
|
||||
it('uses the freshly minted URL', async () => {
|
||||
const getGatewayWsUrl = vi.fn().mockResolvedValue('ws://host/api/ws?ticket=fresh')
|
||||
await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn)).resolves.toBe('ws://host/api/ws?ticket=fresh')
|
||||
expect(getGatewayWsUrl).toHaveBeenCalledOnce()
|
||||
})
|
||||
|
||||
it('throws a reauth error instead of falling back to the stale cached ticket', async () => {
|
||||
const getGatewayWsUrl = vi.fn().mockRejectedValue(new Error('401 cookie expired'))
|
||||
await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn)).rejects.toBeInstanceOf(
|
||||
GatewayReauthRequiredError
|
||||
)
|
||||
})
|
||||
|
||||
it('preserves the underlying mint failure as the cause', async () => {
|
||||
const cause = new Error('401 cookie expired')
|
||||
const getGatewayWsUrl = vi.fn().mockRejectedValue(cause)
|
||||
const error = await resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn).catch(e => e)
|
||||
expect(error).toBeInstanceOf(GatewayReauthRequiredError)
|
||||
expect((error as GatewayReauthRequiredError).cause).toBe(cause)
|
||||
})
|
||||
|
||||
it('throws a reauth error when the preload cannot mint (no method)', async () => {
|
||||
await expect(resolveGatewayWsUrl({}, oauthConn)).rejects.toBeInstanceOf(GatewayReauthRequiredError)
|
||||
})
|
||||
|
||||
it('never returns the stale cached ticket on failure', async () => {
|
||||
const getGatewayWsUrl = vi.fn().mockRejectedValue(new Error('boom'))
|
||||
const result = await resolveGatewayWsUrl({ getGatewayWsUrl }, oauthConn).catch(() => 'threw')
|
||||
expect(result).toBe('threw')
|
||||
expect(result).not.toBe(oauthConn.wsUrl)
|
||||
})
|
||||
})
|
||||
|
||||
describe('token / local mode', () => {
|
||||
it('uses the minted URL when available', async () => {
|
||||
const getGatewayWsUrl = vi.fn().mockResolvedValue('ws://host/api/ws?token=fresh')
|
||||
await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, tokenConn)).resolves.toBe('ws://host/api/ws?token=fresh')
|
||||
})
|
||||
|
||||
it('falls back to the cached URL when minting fails (token is long-lived)', async () => {
|
||||
const getGatewayWsUrl = vi.fn().mockRejectedValue(new Error('transient'))
|
||||
await expect(resolveGatewayWsUrl({ getGatewayWsUrl }, tokenConn)).resolves.toBe(tokenConn.wsUrl)
|
||||
})
|
||||
|
||||
it('falls back to the cached URL when the preload method is absent', async () => {
|
||||
await expect(resolveGatewayWsUrl({}, tokenConn)).resolves.toBe(tokenConn.wsUrl)
|
||||
})
|
||||
|
||||
it('treats a missing authMode as non-oauth (falls back safely)', async () => {
|
||||
await expect(resolveGatewayWsUrl({}, { wsUrl: tokenConn.wsUrl })).resolves.toBe(tokenConn.wsUrl)
|
||||
})
|
||||
})
|
||||
})
|
||||
|
||||
describe('isGatewayReauthRequired', () => {
|
||||
it('detects the dedicated error class', () => {
|
||||
expect(isGatewayReauthRequired(new GatewayReauthRequiredError('x'))).toBe(true)
|
||||
})
|
||||
|
||||
it('detects plain objects tagged with needsOauthLogin (from the main process)', () => {
|
||||
expect(isGatewayReauthRequired({ needsOauthLogin: true })).toBe(true)
|
||||
})
|
||||
|
||||
it('rejects generic errors', () => {
|
||||
expect(isGatewayReauthRequired(new Error('connection closed'))).toBe(false)
|
||||
expect(isGatewayReauthRequired(null)).toBe(false)
|
||||
expect(isGatewayReauthRequired('string')).toBe(false)
|
||||
})
|
||||
})
|
||||
85
apps/desktop/src/lib/gateway-ws-url.ts
Normal file
85
apps/desktop/src/lib/gateway-ws-url.ts
Normal file
@ -0,0 +1,85 @@
|
||||
import type { HermesConnection } from '@/global'
|
||||
|
||||
/**
|
||||
* The desktop main process exposes `getGatewayWsUrl()` to re-mint a WebSocket
|
||||
* URL immediately before every `gateway.connect()`. For OAuth-gated remote
|
||||
* gateways the WS ticket is single-use with a ~30s TTL, so the ticket baked
|
||||
* into the cached `conn.wsUrl` is stale (and, after the first connect, already
|
||||
* consumed). For local/token gateways the URL carries a long-lived token and
|
||||
* never needs re-minting.
|
||||
*
|
||||
* Resolution rules:
|
||||
*
|
||||
* - OAuth: the fresh mint is the *only* viable URL. If it fails, do NOT fall
|
||||
* back to `conn.wsUrl` — that ticket is dead and the connect is guaranteed to
|
||||
* fail with an opaque "connection closed" error. Instead, let the mint error
|
||||
* propagate so the caller can surface the gateway's reauth message
|
||||
* ("session has expired… Sign in again").
|
||||
*
|
||||
* - token / local, or when the preload method is genuinely absent (older
|
||||
* preload shapes): fall back to `conn.wsUrl`. The token URL is long-lived, so
|
||||
* the fallback is safe and preserves compatibility.
|
||||
*
|
||||
* The error thrown for OAuth mint failures is tagged with `needsOauthLogin` so
|
||||
* callers can distinguish "the user must re-authenticate" from a generic
|
||||
* transport failure.
|
||||
*/
|
||||
export interface ResolveGatewayWsUrlDeps {
|
||||
/** `window.hermesDesktop.getGatewayWsUrl`, if the preload exposes it. */
|
||||
getGatewayWsUrl?: () => Promise<string>
|
||||
}
|
||||
|
||||
export class GatewayReauthRequiredError extends Error {
|
||||
readonly needsOauthLogin = true
|
||||
|
||||
constructor(message: string, options?: { cause?: unknown }) {
|
||||
super(message, options)
|
||||
this.name = 'GatewayReauthRequiredError'
|
||||
}
|
||||
}
|
||||
|
||||
export function isGatewayReauthRequired(error: unknown): error is GatewayReauthRequiredError {
|
||||
return (
|
||||
error instanceof GatewayReauthRequiredError ||
|
||||
(typeof error === 'object' && error !== null && (error as { needsOauthLogin?: unknown }).needsOauthLogin === true)
|
||||
)
|
||||
}
|
||||
|
||||
export async function resolveGatewayWsUrl(
|
||||
desktop: ResolveGatewayWsUrlDeps,
|
||||
conn: Pick<HermesConnection, 'authMode' | 'wsUrl'>
|
||||
): Promise<string> {
|
||||
const mint = desktop.getGatewayWsUrl
|
||||
|
||||
if (conn.authMode === 'oauth') {
|
||||
if (!mint) {
|
||||
// OAuth gateway but no way to mint a fresh ticket: the cached ticket is
|
||||
// dead, so connecting with it cannot succeed. Surface a reauth error
|
||||
// rather than silently attempting a doomed connect.
|
||||
throw new GatewayReauthRequiredError(
|
||||
'Your remote gateway session needs to be refreshed. Open Settings → Gateway and click "Sign in" again.'
|
||||
)
|
||||
}
|
||||
|
||||
try {
|
||||
return await mint()
|
||||
} catch (error) {
|
||||
throw new GatewayReauthRequiredError(
|
||||
'Your remote gateway session has expired. Open Settings → Gateway and click "Sign in" again.',
|
||||
{ cause: error }
|
||||
)
|
||||
}
|
||||
}
|
||||
|
||||
// token / local: the URL carries a long-lived token. Re-mint when available
|
||||
// (cheap, keeps parity), but the cached URL is a safe fallback.
|
||||
if (mint) {
|
||||
const fresh = await mint().catch(() => null)
|
||||
|
||||
if (fresh) {
|
||||
return fresh
|
||||
}
|
||||
}
|
||||
|
||||
return conn.wsUrl
|
||||
}
|
||||
Reference in New Issue
Block a user