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
34 changes: 34 additions & 0 deletions .changeset/localization-retry-transient.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
---
"@object-ui/types": minor
"@object-ui/permissions": patch
"@object-ui/console": patch
---

fix(console): `LocalizationFetchProvider` retries a transient `/me/localization` failure instead of degrading for the whole session

`/auth/me/localization` is served by the environment kernel that owns the session
on a multi-tenant host, and a cold one answers `503` + `Retry-After` while it
warms (objectstack#4159). A transient failure is therefore a normal part of a
cold start — not an exception.

The provider made ONE attempt and `.catch()`-ed into silence. So a single 503
during warm-up left currency and locale unset for the **whole session**, silently
and permanently, long after the kernel was ready. Every money field rendered a
plain number and nothing ever tried again.

It now re-attempts a transient failure (`408`, `425`, `429`, `502`, `503`, `504`,
or a thrown fetch), server-stated `Retry-After` first, exponential backoff
otherwise. `401` / `403` / `404` / `500` are real answers about the caller and
still fail on the first attempt.

**It keeps its posture.** This provider is cosmetic, so it renders children
throughout — including mid-retry — and fills the value in if and when an attempt
succeeds. That is the opposite of `MePermissionsProvider`, which is fail-closed
and holds its loading state across the waits. Both are pinned by tests.

The retry PRIMITIVES ("is this transient", "how long to wait", `Retry-After`
parsing) move from `@object-ui/permissions`'s internal module to
`@object-ui/types` — the lowest package both callers can reach — and
`PermissionsFetchError` becomes the generic `HttpFetchError`. One definition of
transient, two policies, rather than a second copy free to drift from the first.
No behaviour change for `MePermissionsProvider`.
120 changes: 120 additions & 0 deletions apps/console/src/LocalizationFetchProvider.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,120 @@
/**
* ObjectUI
* Copyright (c) 2024-present ObjectStack Inc.
*
* `/auth/me/localization` answers `503` + `Retry-After` while a cold environment
* kernel warms (objectstack#4159), so a transient failure is a normal part of a
* cold start on a multi-tenant host. This provider used to make ONE attempt and
* `.catch()` into silence, which meant a single 503 during warm-up degraded
* currency and locale for the whole session — silently, permanently, long after
* the kernel was ready.
*
* These pin the recovery AND the posture it must not lose: unlike the permission
* layer, this one is cosmetic, so it keeps rendering children throughout and
* never gates the app on the answer.
*/

import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, screen, waitFor } from '@testing-library/react';
import { LocalizationFetchProvider } from './LocalizationFetchProvider';

const ENDPOINT = '/api/v1/auth/me/localization';

const ok = (body: unknown) => ({
ok: true,
status: 200,
headers: new Headers(),
json: async () => body,
});
const fail = (status: number, headers: Record<string, string> = {}) => ({
ok: false,
status,
headers: new Headers(headers),
json: async () => ({}),
});

function renderProvider() {
return render(
<LocalizationFetchProvider endpoint={ENDPOINT}>
<span data-testid="child">child</span>
</LocalizationFetchProvider>,
);
}

describe('LocalizationFetchProvider', () => {
beforeEach(() => { vi.spyOn(globalThis, 'fetch' as never); });
afterEach(() => { vi.restoreAllMocks(); });

it('recovers from a warming 503 instead of degrading for the session', async () => {
(globalThis.fetch as never as ReturnType<typeof vi.fn>)
// `Retry-After: 0` keeps the test timer-free while still exercising the
// server-stated-delay path.
.mockResolvedValueOnce(fail(503, { 'Retry-After': '0' }))
.mockResolvedValueOnce(ok({ authenticated: true, currency: 'CNY', locale: 'zh-CN' }));

renderProvider();

await waitFor(() =>
expect((globalThis.fetch as never as ReturnType<typeof vi.fn>)).toHaveBeenCalledTimes(2),
);
// The point of the retry: the tenant default actually arrives.
expect(screen.getByTestId('child')).toBeTruthy();
});

it('retries a thrown fetch (offline / DNS / aborted) too', async () => {
(globalThis.fetch as never as ReturnType<typeof vi.fn>)
.mockRejectedValueOnce(new TypeError('Failed to fetch'))
.mockResolvedValueOnce(ok({ authenticated: true, currency: 'EUR' }));

renderProvider();

// A thrown fetch carries no `Retry-After`, so this one waits out the real
// exponential backoff (BASE_DELAY_MS) rather than a server-stated 0.
await waitFor(
() => expect((globalThis.fetch as never as ReturnType<typeof vi.fn>)).toHaveBeenCalledTimes(2),
{ timeout: 4000 },
);
}, 10_000);

it.each([401, 403, 404, 500])(
'does NOT retry %i — a real answer about this caller will not change',
async (status) => {
(globalThis.fetch as never as ReturnType<typeof vi.fn>).mockResolvedValue(fail(status));

renderProvider();

// Give any (wrongly scheduled) retry a chance to fire before asserting.
await new Promise((r) => setTimeout(r, 50));
expect((globalThis.fetch as never as ReturnType<typeof vi.fn>)).toHaveBeenCalledTimes(1);
},
);

it('never blocks: children render while the fetch is still failing', async () => {
(globalThis.fetch as never as ReturnType<typeof vi.fn>).mockResolvedValue(
fail(503, { 'Retry-After': '0' }),
);

renderProvider();

// The posture that separates this provider from MePermissionsProvider — it
// is cosmetic, so nothing waits on the answer, not even mid-retry.
expect(screen.getByTestId('child')).toBeTruthy();
await new Promise((r) => setTimeout(r, 30));
expect(screen.getByTestId('child')).toBeTruthy();
});

it('stops after the retry budget rather than hammering forever', async () => {
(globalThis.fetch as never as ReturnType<typeof vi.fn>).mockResolvedValue(
fail(503, { 'Retry-After': '0' }),
);

renderProvider();

// 1 attempt + MAX_RETRIES(4); waitFor settles once the count stops moving.
await waitFor(() =>
expect((globalThis.fetch as never as ReturnType<typeof vi.fn>)).toHaveBeenCalledTimes(5),
);
await new Promise((r) => setTimeout(r, 50));
expect((globalThis.fetch as never as ReturnType<typeof vi.fn>)).toHaveBeenCalledTimes(5);
});
});
65 changes: 56 additions & 9 deletions apps/console/src/LocalizationFetchProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,28 @@
* Cosmetic, NOT fail-closed: while loading or on error it renders children with
* an empty value (no tenant default → renderers show a plain number), so a slow
* or missing endpoint never blocks the app.
*
* That posture is why it retries QUIETLY. On a multi-tenant host this endpoint
* is served by the environment kernel that owns the session, and a cold one
* answers `503` + `Retry-After` while it warms (objectstack#4159) — so a
* transient failure is a normal part of a cold start, not an exception. A single
* attempt meant one 503 during warm-up degraded currency and locale for the
* WHOLE session, silently and permanently, long after the kernel was ready.
*
* It shares the "is this transient / how long to wait" primitives with
* `MePermissionsProvider` but not its policy: that one is fail-closed and holds
* its loading state across the waits, this one keeps rendering throughout and
* simply fills the value in if and when an attempt succeeds.
*/
import { useEffect, useState } from 'react';
import { LocalizationProvider, type LocalizationValue } from '@object-ui/i18n';
import {
HttpFetchError,
backoffMs,
isTransientFailure,
retryAfterFrom,
sleep,
} from '@object-ui/types';

interface MeLocalizationResponse {
authenticated?: boolean;
Expand All @@ -18,6 +37,16 @@ interface MeLocalizationResponse {
timezone?: string | null;
}

/**
* Attempts for a transient failure, and the base for the exponential backoff.
* Deliberately more patient than the permission layer's: nothing is waiting on
* this, so it can afford to still be trying when a slow environment finishes
* warming, and a user who never sees a currency symbol is better served by a
* late answer than by no answer.
*/
const MAX_RETRIES = 4;
const BASE_DELAY_MS = 1_000;

export function LocalizationFetchProvider({
endpoint,
children,
Expand All @@ -29,15 +58,33 @@ export function LocalizationFetchProvider({

useEffect(() => {
let cancelled = false;
fetch(endpoint, { credentials: 'include', headers: { Accept: 'application/json' } })
.then((r) => (r.ok ? (r.json() as Promise<MeLocalizationResponse>) : null))
.then((json) => {
if (cancelled || !json) return;
setValue({ currency: json.currency ?? undefined, locale: json.locale ?? undefined });
})
.catch(() => {
/* cosmetic — leave the empty value, renderers show plain numbers */
});

void (async () => {
for (let attempt = 0; attempt <= MAX_RETRIES; attempt++) {
try {
const res = await fetch(endpoint, {
credentials: 'include',
headers: { Accept: 'application/json' },
});
if (!res.ok) throw new HttpFetchError(res.status, retryAfterFrom(res));

const json = (await res.json()) as MeLocalizationResponse;
if (cancelled) return;
setValue({ currency: json.currency ?? undefined, locale: json.locale ?? undefined });
return;
} catch (err) {
// A real answer about this caller (401/403/404/500) will not change on
// a retry, and neither will the last attempt — either way, stop and
// leave the empty value. Cosmetic: renderers show plain numbers.
if (!isTransientFailure(err) || attempt === MAX_RETRIES) return;

const stated = err instanceof HttpFetchError ? err.retryAfterMs : undefined;
await sleep(backoffMs(attempt, BASE_DELAY_MS, stated));
if (cancelled) return;
}
}
})();

return () => {
cancelled = true;
};
Expand Down
15 changes: 8 additions & 7 deletions packages/permissions/src/MePermissionsProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -8,12 +8,12 @@

import React, { useEffect, useMemo, useState, useCallback } from 'react';
import {
PermissionsFetchError,
HttpFetchError,
backoffMs,
isTransientFailure,
parseRetryAfterMs,
retryAfterFrom,
sleep,
} from './retry';
} from '@object-ui/types';
import type {
PermissionAction,
PermissionCheckResult,
Expand Down Expand Up @@ -74,7 +74,7 @@ export interface MePermissionsProviderProps {
errorFallback?: (err: Error, retry: () => void) => React.ReactNode;
/**
* How many times to re-attempt a TRANSIENT failure before giving up — a
* response that says "not now" (`TRANSIENT_STATUS` in `./retry`) or a network
* response that says "not now" (`TRANSIENT_STATUS` in `@object-ui/types`) or a network
* error. `0` disables retrying.
*
* This exists because "not now" is a real answer from this endpoint. On a
Expand Down Expand Up @@ -148,9 +148,10 @@ export function MePermissionsProvider({
headers: { Accept: 'application/json' },
});
if (!res.ok) {
throw new PermissionsFetchError(
throw new HttpFetchError(
res.status,
parseRetryAfterMs(res.headers?.get?.('Retry-After'), Date.now()),
retryAfterFrom(res),
`Permissions endpoint returned ${res.status}`,
);
}
const json = (await res.json()) as MePermissionsResponse;
Expand All @@ -168,7 +169,7 @@ export function MePermissionsProvider({
}
// `loading` stays true across the wait, so the fail-closed loading
// state holds and the recovery is invisible to consumers.
const stated = err instanceof PermissionsFetchError ? err.retryAfterMs : undefined;
const stated = err instanceof HttpFetchError ? err.retryAfterMs : undefined;
await sleep(backoffMs(attempt, retryBaseDelayMs, stated));
if (!live()) return;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ import { describe, it, expect, vi, beforeEach, afterEach } from 'vitest';
import { render, screen, waitFor } from '@testing-library/react';
import React from 'react';
import { MePermissionsProvider } from '../MePermissionsProvider';
import { parseRetryAfterMs } from '../retry';
import { parseRetryAfterMs } from '@object-ui/types';
import { usePermissions } from '../usePermissions';

function Probe() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,23 @@
*/

/**
* Retry primitives for {@link MePermissionsProvider}'s `/me/permissions` fetch.
* Primitives for retrying a TRANSIENT HTTP failure.
*
* Their own module rather than the component's: they are plain functions with
* plain tests, and a component file that also exports helpers breaks fast
* refresh.
* These answer two questions only — "is this worth re-attempting?" and "how long
* until the next attempt?" — and nothing about what to do while waiting. That is
* deliberate: the two providers that use them have opposite postures.
* `MePermissionsProvider` is FAIL-CLOSED and holds its loading state across the
* waits, because rendering a permissive default early is a real hazard;
* `LocalizationFetchProvider` is COSMETIC and never blocks, because a missing
* currency just renders a plain number. One definition of "transient", two
* policies — rather than one policy forced on both, or two copies of the
* definition drifting apart.
*
* They live in this package because it is the lowest one both callers can reach
* (`@object-ui/types` has no `@object-ui` dependencies of its own).
*/

/** Upper bound on any single backoff wait, so a bad `Retry-After` cannot park the UI. */
/** Upper bound on any single backoff wait, so a bad `Retry-After` cannot park a caller. */
export const MAX_RETRY_DELAY_MS = 30_000;

/**
Expand All @@ -24,10 +33,10 @@ export const MAX_RETRY_DELAY_MS = 30_000;
* - `408` / `425` — the request itself did not land.
* - `429` — rate limited; `Retry-After` usually accompanies it.
* - `502` / `503` / `504` — the upstream is absent, warming or timing out.
* `503` is the load-bearing one here: on a multi-tenant host this endpoint is
* served by the environment kernel that owns the session, and the framework
* answers `503` + `Retry-After` while a cold one is still being built
* (objectstack#4159).
* `503` is the load-bearing one: on a multi-tenant host the `/auth/me/*`
* endpoints are served by the environment kernel that owns the session, and
* the framework answers `503` + `Retry-After` while a cold one is still being
* built (objectstack#4159). Both providers see it, so both must survive it.
*
* Deliberately NOT retried: `401` / `403` (a real answer about this caller),
* `404` (no such endpoint — retrying cannot conjure one) and `500` (a genuine
Expand All @@ -36,10 +45,14 @@ export const MAX_RETRY_DELAY_MS = 30_000;
export const TRANSIENT_STATUS: ReadonlySet<number> = new Set([408, 425, 429, 502, 503, 504]);

/** An `Error` that also carries the HTTP status it came from. */
export class PermissionsFetchError extends Error {
constructor(readonly status: number, readonly retryAfterMs?: number) {
super(`Permissions endpoint returned ${status}`);
this.name = 'PermissionsFetchError';
export class HttpFetchError extends Error {
constructor(
readonly status: number,
readonly retryAfterMs?: number,
message?: string,
) {
super(message ?? `Endpoint returned ${status}`);
this.name = 'HttpFetchError';
}
}

Expand All @@ -49,15 +62,18 @@ export class PermissionsFetchError extends Error {
* never got an answer at all.
*/
export function isTransientFailure(err: unknown): boolean {
return err instanceof PermissionsFetchError ? TRANSIENT_STATUS.has(err.status) : true;
return err instanceof HttpFetchError ? TRANSIENT_STATUS.has(err.status) : true;
}

/**
* `Retry-After` in ms, or `undefined` when absent/unparseable. Accepts both wire
* forms (delta-seconds and an HTTP-date) and clamps to {@link MAX_RETRY_DELAY_MS}
* so a hostile or buggy value cannot park the UI on its loading state for hours.
* so a hostile or buggy value cannot park a caller for hours.
*/
export function parseRetryAfterMs(value: string | null | undefined, nowMs: number): number | undefined {
export function parseRetryAfterMs(
value: string | null | undefined,
nowMs: number,
): number | undefined {
if (!value) return undefined;
const trimmed = value.trim();
if (!trimmed) return undefined;
Expand All @@ -81,3 +97,8 @@ export function backoffMs(attempt: number, baseDelayMs: number, statedMs?: numbe

export const sleep = (ms: number): Promise<void> =>
new Promise<void>((resolve) => setTimeout(resolve, ms));

/** Read `Retry-After` off a response without assuming `headers` exists (test doubles often omit it). */
export function retryAfterFrom(res: { headers?: { get?(name: string): string | null } }): number | undefined {
return parseRetryAfterMs(res.headers?.get?.('Retry-After'), Date.now());
}
Loading
Loading