🐛 fix(desktop): trace Session Expired cause and resume onboarding at Login (#15604)

- Carry a `reason` payload on the `authorizationRequired` IPC event so the
  cause behind the Session Expired modal (proxy 401, refresh non-retryable,
  startup proactive refresh exception, etc.) lands in `electron-log` and the
  renderer debug namespace for postmortem.
- On 401 + `X-Auth-Required`, enrich the reason with `hadToken`, the upstream
  `www-authenticate` header and a truncated body snippet so OAuth/tRPC error
  details are captured without consuming the forwarded stream.
- Fix returning users (token refresh failed -> active=false -> relaunch)
  landing on the Welcome screen of desktop onboarding. Persist an
  `everCompleted` flag in localStorage and resume at the Login screen for
  anyone who has already completed onboarding once.
- Extract the screen-resolution logic into a pure `resolveInitialScreen`
  helper with unit tests; cover the new storage flag and reason payload in
  AuthCtr / BackendProxy tests.
This commit is contained in:
Innei
2026-06-10 01:06:00 +08:00
committed by GitHub
parent c329696dc2
commit 991c2f79e8
11 changed files with 328 additions and 36 deletions
+14 -8
View File
@@ -321,7 +321,9 @@ export default class AuthCtr extends ControllerModule {
this.stopAutoRefresh();
await this.remoteServerConfigCtr.clearTokens();
await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
this.broadcastAuthorizationRequired();
this.broadcastAuthorizationRequired(
`auto-refresh:non_retryable ${result.error ?? ''}`.trim(),
);
} else {
// For other errors (after retries exhausted), log but don't clear tokens immediately
// The next refresh cycle will retry
@@ -432,7 +434,7 @@ export default class AuthCtr extends ControllerModule {
this.stopAutoRefresh();
await this.remoteServerConfigCtr.clearTokens();
await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
this.broadcastAuthorizationRequired();
this.broadcastAuthorizationRequired(`refresh:non_retryable ${result.error ?? ''}`.trim());
} else {
// For transient errors, don't clear tokens - allow manual retry
logger.warn('Refresh failed but error may be transient, tokens preserved for retry');
@@ -450,7 +452,7 @@ export default class AuthCtr extends ControllerModule {
this.stopAutoRefresh();
await this.remoteServerConfigCtr.clearTokens();
await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
this.broadcastAuthorizationRequired();
this.broadcastAuthorizationRequired(`refresh:exception ${errorMessage}`);
}
return { error: errorMessage, success: false };
@@ -618,15 +620,17 @@ export default class AuthCtr extends ControllerModule {
}
/**
* Broadcast authorization required event
* Broadcast authorization required event.
* `reason` is a short tag (e.g. `refresh:invalid_grant`, `startup:non_retryable`)
* recorded so the renderer can log why the Session Expired modal appeared.
*/
private broadcastAuthorizationRequired() {
logger.debug('Broadcasting authorizationRequired event to all windows');
private broadcastAuthorizationRequired(reason: string) {
logger.info(`Broadcasting authorizationRequired event (reason=${reason})`);
const allWindows = BrowserWindow.getAllWindows();
for (const win of allWindows) {
if (!win.isDestroyed()) {
win.webContents.send('authorizationRequired');
win.webContents.send('authorizationRequired', { reason });
}
}
}
@@ -751,7 +755,9 @@ export default class AuthCtr extends ControllerModule {
logger.warn('Non-retryable error during proactive refresh, clearing tokens');
await this.remoteServerConfigCtr.clearTokens();
await this.remoteServerConfigCtr.setRemoteServerConfig({ active: false });
this.broadcastAuthorizationRequired();
this.broadcastAuthorizationRequired(
`startup:non_retryable ${refreshResult.error ?? ''}`.trim(),
);
} else {
// For transient errors, still start auto-refresh timer to retry later
logger.warn('Transient error during proactive refresh, will retry via auto-refresh');
@@ -797,7 +797,12 @@ describe('AuthCtr', () => {
expect(mockRemoteServerConfigCtr.setRemoteServerConfig).toHaveBeenCalledWith({
active: false,
});
expect(mockWindow.webContents.send).toHaveBeenCalledWith('authorizationRequired');
expect(mockWindow.webContents.send).toHaveBeenCalledWith(
'authorizationRequired',
expect.objectContaining({
reason: expect.stringContaining('startup:non_retryable'),
}),
);
});
it('should preserve tokens on transient error', async () => {
@@ -32,22 +32,30 @@ export class BackendProxyProtocolManager {
private readonly logger = createLogger('core:BackendProxyProtocolManager');
private authRequiredDebounceTimer: NodeJS.Timeout | null = null;
private pendingAuthRequiredReason: string | null = null;
private static readonly AUTH_REQUIRED_DEBOUNCE_MS = 1000;
private notifyAuthorizationRequired() {
private notifyAuthorizationRequired(reason: string) {
// Trailing-edge debounce: coalesce rapid 401 bursts and fire AFTER the burst settles.
// This ensures the IPC event is sent after the renderer has had time to mount listeners.
// The most recent reason wins — within a burst they almost always describe the same cause.
this.pendingAuthRequiredReason = reason;
if (this.authRequiredDebounceTimer) {
clearTimeout(this.authRequiredDebounceTimer);
}
this.authRequiredDebounceTimer = setTimeout(() => {
this.authRequiredDebounceTimer = null;
const finalReason = this.pendingAuthRequiredReason ?? reason;
this.pendingAuthRequiredReason = null;
this.logger.info(`Broadcasting authorizationRequired (reason=${finalReason})`);
const allWindows = BrowserWindow.getAllWindows();
for (const win of allWindows) {
if (!win.isDestroyed()) {
win.webContents.send('authorizationRequired');
win.webContents.send('authorizationRequired', { reason: finalReason });
}
}
}, BackendProxyProtocolManager.AUTH_REQUIRED_DEBOUNCE_MS);
@@ -196,7 +204,32 @@ export class BackendProxyProtocolManager {
// Other failures keep 401 without this header (e.g., invalid API keys) and must not notify here.
const authRequired = upstreamResponse.headers.get(AUTH_REQUIRED_HEADER) === 'true';
if (authRequired) {
this.notifyAuthorizationRequired();
const pathTag = (() => {
try {
return new URL(rewrittenUrl).pathname;
} catch {
return rewrittenUrl;
}
})();
const sourceTag = context.source ? `${context.source}:` : '';
const wwwAuth = upstreamResponse.headers.get('www-authenticate') ?? '';
// Clone before forwarding the body downstream — the original stream stays
// intact for the renderer. Body snippet is truncated to keep logs small
// and to avoid leaking large payloads if the server ever returns one.
let bodySnippet: string;
try {
bodySnippet = (await upstreamResponse.clone().text()).slice(0, 300).replaceAll(/\s+/g, ' ');
} catch (error) {
bodySnippet = `<body-read-failed:${error instanceof Error ? error.message : 'unknown'}>`;
}
const parts = [
`proxy:${sourceTag}status=${upstreamResponse.status}`,
`${request.method} ${pathTag}`,
`hadToken=${Boolean(token)}`,
];
if (wwwAuth) parts.push(`wwwAuth=${wwwAuth}`);
if (bodySnippet) parts.push(`body=${bodySnippet}`);
this.notifyAuthorizationRequired(parts.join(' '));
}
return new Response(upstreamResponse.body, {
@@ -258,7 +258,63 @@ describe('BackendProxyProtocolManager', () => {
expect(send).not.toHaveBeenCalled();
await vi.advanceTimersByTimeAsync(1000);
expect(send).toHaveBeenCalledWith('authorizationRequired');
expect(send).toHaveBeenCalledWith(
'authorizationRequired',
expect.objectContaining({
reason: expect.stringContaining('status=207'),
}),
);
});
it('captures www-authenticate, body snippet and hadToken in reason on 401', async () => {
vi.useFakeTimers();
const send = vi.fn();
vi.mocked(BrowserWindow.getAllWindows).mockReturnValue([
{ isDestroyed: () => false, webContents: { send } },
] as any);
const manager = new BackendProxyProtocolManager();
const session = {} as any;
const upstreamBody = JSON.stringify({
error: { json: { data: { code: 'UNAUTHORIZED' }, message: 'token expired at 2026-06-09' } },
});
const headers = new Headers({
[AUTH_REQUIRED_HEADER]: 'true',
'Content-Type': 'application/json',
'www-authenticate': 'Bearer error="invalid_token", error_description="expired"',
});
const fetchMock = vi.fn<FetchMock>(
async () => new Response(upstreamBody, { headers, status: 401, statusText: 'Unauthorized' }),
);
vi.stubGlobal('fetch', fetchMock as any);
manager.registerWithRemoteBaseUrl(session, {
getAccessToken: async () => 'fake-token',
getRemoteBaseUrl: async () => 'https://remote.example.com',
});
const response = await manager.proxy(
{
headers: new Headers(),
method: 'POST',
url: 'app://renderer/trpc/lambda/me',
} as any,
session,
);
// Original body is still readable by the downstream caller — clone() must not consume it.
expect(await response!.text()).toBe(upstreamBody);
await vi.advanceTimersByTimeAsync(1000);
expect(send).toHaveBeenCalledTimes(1);
const [, payload] = send.mock.calls[0];
expect(payload.reason).toContain('status=401');
expect(payload.reason).toContain('POST /trpc/lambda/me');
expect(payload.reason).toContain('hadToken=true');
expect(payload.reason).toContain('wwwAuth=Bearer error="invalid_token"');
expect(payload.reason).toContain('UNAUTHORIZED');
expect(payload.reason).toContain('token expired');
});
describe('createAppRequestInterceptor', () => {