Skip to content

Commit 2307b52

Browse files
authored
fix(permissions,console): retry a transient /me/permissions failure instead of stranding the app on its loading state (#3050) (#3052)
"Not now" is a real answer from this endpoint. On a multi-tenant host it is served by the environment kernel that owns the session, and a COLD one answers `503` + `Retry-After` while it warms (objectstack#4159 / cloud#927 — `KernelWarmingError` states ~15s). The provider treated that like any other failure: it set `error`. And the render branch falls back to `loadingFallback` for the error state whenever no `errorFallback` is given — exactly how the console wires it. So a tenant opening the console during a cold start sat on a spinner indefinitely, with a `retry` no path could reach. Both halves are needed; neither alone is enough. 1. The fetch re-attempts a TRANSIENT failure — 408, 425, 429, 502, 503, 504, or a thrown fetch (offline / DNS / aborted), which never got an answer at all. A server-stated `Retry-After` wins over the exponential backoff (both wire forms parsed, clamped to 30s so a hostile value cannot park the UI). `loading` stays true across the waits, so the fail-closed state holds and consumers never see a permissive flash mid-recovery. 401/403/404/500 are real answers about the caller and fail on the first attempt, exactly as before. New props, both defaulted so no call site changes: `maxRetries` (3; `0` restores the old behaviour) and `retryBaseDelayMs` (500). 2. The console passes an `errorFallback`. Retrying narrows the window but cannot close it — a build slower than the retry budget still lands in the error state. It now renders `<LoadingScreen error onRetry />`, the affordance that component has carried all along and nobody wired. Also fixes a latent race the retries made much wider: the in-flight fetch is cancelled when the effect tears down, so a slow answer for a previous `endpoint` or `fetcher` can no longer overwrite a fast answer for the current one. The retry primitives live in an internal `./retry` module (not exported from the package root) — plain functions with plain tests, and a component file that also exports helpers breaks fast refresh.
1 parent 68ef584 commit 2307b52

5 files changed

Lines changed: 407 additions & 20 deletions

File tree

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
---
2+
"@object-ui/permissions": minor
3+
"@object-ui/console": patch
4+
---
5+
6+
fix(permissions,console): `MePermissionsProvider` retries a transient `/me/permissions` failure instead of stranding the app on its loading state
7+
8+
"Not now" is a real answer from this endpoint. On a multi-tenant host it is served
9+
by the environment kernel that owns the session, and a COLD one answers `503` +
10+
`Retry-After` while it warms (objectstack#4159 / cloud#927). The provider treated
11+
that like any other failure: it set `error` — and a consumer that passes no
12+
`errorFallback` renders `loadingFallback` for the error state too. The console
13+
does exactly that (`loadingFallback={<LoadingScreen />}`, no `errorFallback`), so
14+
the app sat on its spinner indefinitely, with a `retry` nobody could reach.
15+
16+
The fetch now re-attempts a **transient** failure — `408`, `425`, `429`, `502`,
17+
`503`, `504`, or a thrown fetch (offline / DNS / aborted), which never got an
18+
answer at all. A server-stated `Retry-After` wins over the exponential backoff
19+
(both wire forms are read, and clamped to 30s so a hostile value cannot park the
20+
UI); otherwise the delay doubles from `retryBaseDelayMs`. `loading` stays true
21+
across the waits, so the fail-closed loading state holds and consumers never see
22+
a permissive flash mid-recovery.
23+
24+
Unchanged for a real answer about the caller: `401`, `403`, `404` and `500` fail
25+
on the first attempt exactly as before. `500` is deliberately not retried — a
26+
genuine server fault neither benefits from hammering nor should be hidden behind
27+
a spinner.
28+
29+
**New props**, both optional and defaulted so no call site needs to change:
30+
31+
- `maxRetries` (default `3`) — `0` restores the previous single-attempt
32+
behaviour.
33+
- `retryBaseDelayMs` (default `500`) — base for the exponential backoff.
34+
35+
Also fixes a latent race the retries made much wider: the in-flight fetch is now
36+
cancelled when the effect tears down, so a slow answer for a previous `endpoint`
37+
or `fetcher` can no longer overwrite a fast answer for the current one. The retry
38+
primitives (`parseRetryAfterMs`, `backoffMs`, `isTransientFailure`,
39+
`TRANSIENT_STATUS`, `PermissionsFetchError`) live in a new internal `./retry`
40+
module — not exported from the package root.
41+
42+
**The console now passes an `errorFallback`.** Retrying narrows the window but
43+
cannot close it — a kernel build slower than the retry budget still lands in the
44+
error state, and rendering `loadingFallback` there is what produced the eternal
45+
spinner. It now renders `<LoadingScreen error={...} onRetry={retry} />`, using the
46+
error + retry affordance that component has carried all along, so a user is never
47+
left with a spinner and no way forward.

apps/console/src/AppContent.tsx

Lines changed: 12 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -132,7 +132,18 @@ export function AppContent() {
132132
// rendering fell open (restricted fields rendered editable).
133133
const authFetch = useMemo(() => createAuthenticatedFetch(), []);
134134
return (
135-
<MePermissionsProvider endpoint={endpoint} fetcher={authFetch} loadingFallback={<LoadingScreen />}>
135+
<MePermissionsProvider
136+
endpoint={endpoint}
137+
fetcher={authFetch}
138+
loadingFallback={<LoadingScreen />}
139+
// Without this, the provider's error state renders `loadingFallback` too —
140+
// an eternal spinner with a `retry` nobody can reach. The provider already
141+
// re-attempts a transient failure (a cold environment kernel answers 503 +
142+
// `Retry-After` while it warms, objectstack#4159); this is what the user
143+
// sees once those are exhausted, and `LoadingScreen` has carried the
144+
// error + retry affordance all along.
145+
errorFallback={(err, retry) => <LoadingScreen error={err.message} onRetry={retry} />}
146+
>
136147
<LocalizationFetchProvider endpoint={localizationEndpoint}>
137148
<UploadProvider adapter={uploadAdapter}>
138149
<DefaultAppContent extraRoutes={systemRoutes} extraRoutesNoApp={systemRoutes} />

packages/permissions/src/MePermissionsProvider.tsx

Lines changed: 88 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,13 @@
77
*/
88

99
import React, { useEffect, useMemo, useState, useCallback } from 'react';
10+
import {
11+
PermissionsFetchError,
12+
backoffMs,
13+
isTransientFailure,
14+
parseRetryAfterMs,
15+
sleep,
16+
} from './retry';
1017
import type {
1118
PermissionAction,
1219
PermissionCheckResult,
@@ -65,6 +72,27 @@ export interface MePermissionsProviderProps {
6572
loadingFallback?: React.ReactNode;
6673
/** Rendered when load fails */
6774
errorFallback?: (err: Error, retry: () => void) => React.ReactNode;
75+
/**
76+
* How many times to re-attempt a TRANSIENT failure before giving up — a
77+
* response that says "not now" (`TRANSIENT_STATUS` in `./retry`) or a network
78+
* error. `0` disables retrying.
79+
*
80+
* This exists because "not now" is a real answer from this endpoint. On a
81+
* multi-tenant host it is served by the environment kernel that owns the
82+
* session, and a cold one answers `503` + `Retry-After` while it warms
83+
* (objectstack#4159). Without a retry the provider keeps `loadingFallback`
84+
* on screen forever, because a consumer that passes no `errorFallback` — the
85+
* console does exactly that — renders the loading node for the error state
86+
* too, and nothing ever calls `retry`.
87+
*
88+
* @default 3
89+
*/
90+
maxRetries?: number;
91+
/**
92+
* Base for the exponential backoff between retries, in ms. A `Retry-After`
93+
* header on the response wins over it. @default 500
94+
*/
95+
retryBaseDelayMs?: number;
6896
/** Children */
6997
children: React.ReactNode;
7098
}
@@ -88,34 +116,75 @@ export function MePermissionsProvider({
88116
initialPermissions,
89117
loadingFallback = null,
90118
errorFallback,
119+
maxRetries = 3,
120+
retryBaseDelayMs = 500,
91121
children,
92122
}: MePermissionsProviderProps) {
93123
const [data, setData] = useState<MePermissionsResponse | null>(initialPermissions ?? null);
94124
const [error, setError] = useState<Error | null>(null);
95125
const [loading, setLoading] = useState(!initialPermissions);
96126

97-
const fetchPermissions = useCallback(async () => {
98-
setLoading(true);
99-
setError(null);
100-
try {
127+
/**
128+
* Run the fetch, re-attempting a transient failure.
129+
*
130+
* `token` cancels an in-flight attempt. Retries put real time (a backoff, or
131+
* a server-stated `Retry-After`) between the request and the `setState`, and
132+
* during that window the effect may be torn down — by an unmount, or by
133+
* `endpoint`/`fetcher` changing. Without the token a superseded attempt would
134+
* still land, so a slow answer for the OLD endpoint could overwrite a fast
135+
* answer for the new one.
136+
*/
137+
const fetchPermissions = useCallback(
138+
async (token?: { cancelled: boolean }) => {
139+
const live = () => !token?.cancelled;
140+
setLoading(true);
141+
setError(null);
101142
const doFetch = fetcher ?? fetch;
102-
const res = await doFetch(endpoint, {
103-
credentials: 'include',
104-
headers: { Accept: 'application/json' },
105-
});
106-
if (!res.ok) throw new Error(`Permissions endpoint returned ${res.status}`);
107-
const json = (await res.json()) as MePermissionsResponse;
108-
setData(json);
109-
} catch (e) {
110-
setError(e instanceof Error ? e : new Error(String(e)));
111-
} finally {
112-
setLoading(false);
113-
}
114-
}, [endpoint, fetcher]);
143+
const attempts = Math.max(0, maxRetries) + 1;
144+
for (let attempt = 0; attempt < attempts; attempt++) {
145+
try {
146+
const res = await doFetch(endpoint, {
147+
credentials: 'include',
148+
headers: { Accept: 'application/json' },
149+
});
150+
if (!res.ok) {
151+
throw new PermissionsFetchError(
152+
res.status,
153+
parseRetryAfterMs(res.headers?.get?.('Retry-After'), Date.now()),
154+
);
155+
}
156+
const json = (await res.json()) as MePermissionsResponse;
157+
if (!live()) return;
158+
setData(json);
159+
setLoading(false);
160+
return;
161+
} catch (e) {
162+
const err = e instanceof Error ? e : new Error(String(e));
163+
if (!isTransientFailure(err) || attempt === attempts - 1) {
164+
if (!live()) return;
165+
setError(err);
166+
setLoading(false);
167+
return;
168+
}
169+
// `loading` stays true across the wait, so the fail-closed loading
170+
// state holds and the recovery is invisible to consumers.
171+
const stated = err instanceof PermissionsFetchError ? err.retryAfterMs : undefined;
172+
await sleep(backoffMs(attempt, retryBaseDelayMs, stated));
173+
if (!live()) return;
174+
}
175+
}
176+
},
177+
[endpoint, fetcher, maxRetries, retryBaseDelayMs],
178+
);
179+
180+
/** The `retry` handed to `errorFallback` — uncancelled, it is user-initiated. */
181+
const retry = useCallback(() => { void fetchPermissions(); }, [fetchPermissions]);
115182

116183
useEffect(() => {
117184
if (initialPermissions) return;
118-
void fetchPermissions();
185+
const token = { cancelled: false };
186+
void fetchPermissions(token);
187+
return () => { token.cancelled = true; };
119188
}, [fetchPermissions, initialPermissions]);
120189

121190
const checkField = useCallback(
@@ -232,7 +301,7 @@ export function MePermissionsProvider({
232301

233302
if (loading && !data) return <>{loadingFallback}</>;
234303
if (error && !data) {
235-
if (errorFallback) return <>{errorFallback(error, fetchPermissions)}</>;
304+
if (errorFallback) return <>{errorFallback(error, retry)}</>;
236305
return <>{loadingFallback}</>;
237306
}
238307

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
/**
2+
* ObjectUI
3+
* Copyright (c) 2024-present ObjectStack Inc.
4+
*
5+
* "Not now" is a real answer from `/me/permissions`, and it used to strand the
6+
* console.
7+
*
8+
* On a multi-tenant host the endpoint is served by the environment kernel that
9+
* owns the session, and a COLD one answers `503` + `Retry-After` while it warms
10+
* (objectstack#4159 / cloud#927). The provider treated that like any other
11+
* failure: it set `error`, and a consumer that passes no `errorFallback` — the
12+
* console passes only `loadingFallback={<LoadingScreen />}` — renders the
13+
* loading node for the error state too. So the app sat on its spinner forever,
14+
* with a `retry` nobody could reach.
15+
*
16+
* These pin the retry: transient answers are re-attempted (server-stated delay
17+
* first), real answers about the caller are not, and the fail-closed loading
18+
* state holds throughout rather than flashing permissive.
19+
*/
20+
21+
import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
22+
import { render, screen, waitFor } from '@testing-library/react';
23+
import React from 'react';
24+
import { MePermissionsProvider } from '../MePermissionsProvider';
25+
import { parseRetryAfterMs } from '../retry';
26+
import { usePermissions } from '../usePermissions';
27+
28+
function Probe() {
29+
const { isLoaded, check } = usePermissions();
30+
return (
31+
<div>
32+
<span data-testid="loaded">{String(isLoaded)}</span>
33+
<span data-testid="edit">{String(check('account', 'update').allowed)}</span>
34+
</div>
35+
);
36+
}
37+
38+
const PAYLOAD = {
39+
authenticated: true,
40+
userId: 'u1',
41+
tenantId: 't1',
42+
roles: [],
43+
permissionSets: ['member_default'],
44+
objects: { account: { allowRead: true, allowEdit: false } },
45+
fields: {},
46+
};
47+
48+
const ok = () => ({ ok: true, status: 200, headers: new Headers(), json: async () => PAYLOAD });
49+
const fail = (status: number, headers: Record<string, string> = {}) => ({
50+
ok: false,
51+
status,
52+
headers: new Headers(headers),
53+
json: async () => ({}),
54+
});
55+
56+
/** Render with retries fast enough that the tests need no timer control. */
57+
const renderProvider = (props: Record<string, unknown> = {}) =>
58+
render(
59+
<MePermissionsProvider retryBaseDelayMs={1} {...props}>
60+
<Probe />
61+
</MePermissionsProvider>,
62+
);
63+
64+
describe('MePermissionsProvider retries a transient failure', () => {
65+
beforeEach(() => { vi.spyOn(global, 'fetch' as any); });
66+
afterEach(() => { vi.restoreAllMocks(); });
67+
68+
it('recovers from a warming 503 and serves the real answer', async () => {
69+
(global.fetch as any)
70+
.mockResolvedValueOnce(fail(503, { 'Retry-After': '0' }))
71+
.mockResolvedValueOnce(ok());
72+
73+
renderProvider();
74+
75+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
76+
// The permission answer really arrived — not the anonymous fail-open.
77+
expect(screen.getByTestId('edit').textContent).toBe('false');
78+
expect((global.fetch as any)).toHaveBeenCalledTimes(2);
79+
});
80+
81+
it('prefers the server-stated Retry-After over its own backoff', async () => {
82+
// Timer-free proof: the backoff base is 100s, so if the header were ignored
83+
// this test could not finish. `Retry-After: 0` says "now".
84+
(global.fetch as any)
85+
.mockResolvedValueOnce(fail(503, { 'Retry-After': '0' }))
86+
.mockResolvedValueOnce(ok());
87+
88+
renderProvider({ retryBaseDelayMs: 100_000 });
89+
90+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
91+
});
92+
93+
it('retries a thrown fetch (offline / DNS / aborted) too', async () => {
94+
(global.fetch as any)
95+
.mockRejectedValueOnce(new TypeError('Failed to fetch'))
96+
.mockResolvedValueOnce(ok());
97+
98+
renderProvider();
99+
100+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
101+
});
102+
103+
it('holds the fail-closed loading state while retrying — never a permissive flash', async () => {
104+
(global.fetch as any)
105+
.mockResolvedValueOnce(fail(503))
106+
.mockResolvedValueOnce(fail(503))
107+
.mockResolvedValueOnce(ok());
108+
109+
renderProvider({ loadingFallback: <span data-testid="spinner" /> });
110+
111+
// Children (and therefore any permissive default) are not rendered yet.
112+
expect(screen.getByTestId('spinner')).toBeTruthy();
113+
expect(screen.queryByTestId('loaded')).toBeNull();
114+
await waitFor(() => expect(screen.getByTestId('loaded').textContent).toBe('true'));
115+
});
116+
117+
it('gives up after maxRetries and surfaces the error', async () => {
118+
(global.fetch as any).mockResolvedValue(fail(503));
119+
120+
const errorFallback = vi.fn((err: Error) => <span data-testid="err">{err.message}</span>);
121+
renderProvider({ maxRetries: 2, errorFallback });
122+
123+
await waitFor(() => expect(screen.getByTestId('err')).toBeTruthy());
124+
expect(screen.getByTestId('err').textContent).toContain('503');
125+
expect((global.fetch as any)).toHaveBeenCalledTimes(3); // 1 attempt + 2 retries
126+
});
127+
});
128+
129+
describe('MePermissionsProvider does NOT retry a real answer about the caller', () => {
130+
beforeEach(() => { vi.spyOn(global, 'fetch' as any); });
131+
afterEach(() => { vi.restoreAllMocks(); });
132+
133+
it.each([401, 403, 404, 500])('gives up immediately on %i', async (status) => {
134+
(global.fetch as any).mockResolvedValue(fail(status));
135+
136+
const errorFallback = vi.fn((err: Error) => <span data-testid="err">{err.message}</span>);
137+
renderProvider({ errorFallback });
138+
139+
await waitFor(() => expect(screen.getByTestId('err')).toBeTruthy());
140+
expect((global.fetch as any)).toHaveBeenCalledTimes(1);
141+
});
142+
143+
it('maxRetries: 0 disables retrying entirely', async () => {
144+
(global.fetch as any).mockResolvedValue(fail(503));
145+
146+
const errorFallback = vi.fn((err: Error) => <span data-testid="err">{err.message}</span>);
147+
renderProvider({ maxRetries: 0, errorFallback });
148+
149+
await waitFor(() => expect(screen.getByTestId('err')).toBeTruthy());
150+
expect((global.fetch as any)).toHaveBeenCalledTimes(1);
151+
});
152+
});
153+
154+
describe('parseRetryAfterMs', () => {
155+
const NOW = Date.parse('2026-07-30T12:00:00Z');
156+
157+
it('reads delta-seconds', () => {
158+
expect(parseRetryAfterMs('4', NOW)).toBe(4000);
159+
expect(parseRetryAfterMs('0', NOW)).toBe(0);
160+
expect(parseRetryAfterMs(' 7 ', NOW)).toBe(7000);
161+
});
162+
163+
it('reads an HTTP-date, relative to now', () => {
164+
expect(parseRetryAfterMs('Thu, 30 Jul 2026 12:00:03 GMT', NOW)).toBe(3000);
165+
});
166+
167+
it('clamps a far-future value so it cannot park the UI', () => {
168+
expect(parseRetryAfterMs('999999', NOW)).toBe(30_000);
169+
expect(parseRetryAfterMs('Fri, 31 Jul 2026 12:00:00 GMT', NOW)).toBe(30_000);
170+
});
171+
172+
it('ignores absent, empty, unparseable and past values', () => {
173+
for (const v of [null, undefined, '', ' ', 'soon', 'Thu, 30 Jul 2026 11:59:00 GMT']) {
174+
expect(parseRetryAfterMs(v, NOW), String(v)).toBeUndefined();
175+
}
176+
});
177+
});

0 commit comments

Comments
 (0)