Skip to content
Draft
Show file tree
Hide file tree
Changes from 5 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
2 changes: 2 additions & 0 deletions static/app/__mocks__/api.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,8 @@ const RealApi = jest.requireActual<typeof ApiNamespace>('sentry/api');

export const initApiClientErrorHandling = RealApi.initApiClientErrorHandling;
export const hasProjectBeenRenamed = RealApi.hasProjectBeenRenamed;
export const isSimilarOrigin = RealApi.isSimilarOrigin;
export const resolveHostname = RealApi.resolveHostname;

const respond = (
asyncDelay: AsyncDelay,
Expand Down
728 changes: 728 additions & 0 deletions static/app/api.analysis.md

Large diffs are not rendered by default.

8 changes: 7 additions & 1 deletion static/app/main.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -17,12 +17,18 @@ import {SENTRY_RELEASE_VERSION, USE_TANSTACK_DEVTOOL} from 'sentry/constants';
import {preload} from 'sentry/router/preload';
import {RouteConfigProvider} from 'sentry/router/routeConfigContext';
import {routes} from 'sentry/router/routes';
import {configureSentryCellFetch} from 'sentry/utils/api/sentryCellFetch';
import {createDefaultErrorHandlers} from 'sentry/utils/api/sentryCellFetchErrorHandlers';
import {createReactRouter3Navigate} from 'sentry/utils/useNavigate';

function buildRouter() {
const sentryCreateBrowserRouter = wrapCreateBrowserRouterV6(createBrowserRouter);
const router = sentryCreateBrowserRouter(routes());
setApiNavigate(createReactRouter3Navigate(router));
const navigate = createReactRouter3Navigate(router);
setApiNavigate(navigate);
configureSentryCellFetch({
errorHandlers: createDefaultErrorHandlers({navigate}),
});

return router;
}
Expand Down
85 changes: 85 additions & 0 deletions static/app/sentryCellFetch.analysis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,85 @@
# `sentryCellFetch` vs `api.requestPromise` — Behavioral Comparison

## The Key Difference: What Happens When an Error Handler Suppresses an Error

When a registered error handler (auth redirect, SSO, project rename, etc.) handles a non-2xx response and signals "I handled this," the two systems diverge:

### `api.requestPromise()` — Promise Hangs Forever

In `api.tsx` line 646–653, when a `globalErrorHandler` returns `true`:

```
ok = false path:
1. successHandler is NOT called (we're in the error branch)
2. globalErrorHandlers run — one returns true → shouldSkipErrorHandler = true
3. errorHandler is NOT called (skipped by the guard)
4. completeHandler IS called (but requestPromise doesn't use it)
```

Since `requestPromise` wraps `request()` by overriding only `success` (→ resolve) and `error` (→ reject), and neither callback fires, **the returned Promise never settles**. It hangs indefinitely.

In practice this is "fine" because the handler is doing a hard redirect (`window.location.assign`, `window.location.reload`, or SPA navigate to `/auth/login/`), so the hanging promise is abandoned when the page navigates away.

### `sentryCellFetch()` — Promise Resolves with `{json: undefined}`

In `sentryCellFetch.tsx` `handleErrorResponse()` (line 192–238), when a handler suppresses an error it **returns** a value:

```ts
// e.g., onAuthError returns true
if (responseMeta.status === 401 && errorHandlers?.onAuthError?.(responseMeta, options)) {
return {headers: buildResponseHeaders(response), json: undefined as unknown};
}
```

The promise **resolves successfully** with `{headers: {...}, json: undefined}`.

## Why This Matters

| Scenario | `requestPromise` | `sentryCellFetch` |
| ------------------- | ---------------------------------- | -------------------------------------- |
| Auth redirect (401) | Promise hangs; page navigates away | Promise resolves with `undefined` data |
| SSO required | Promise hangs; browser redirects | Promise resolves with `undefined` data |
| Project renamed | Promise hangs; redirect happens | Promise resolves with `undefined` data |
| Member over limit | Promise hangs; SPA navigates | Promise resolves with `undefined` data |

### Consequences for React Query

With `sentryCellFetch` as a `queryFn`, a suppressed error means React Query sees a **successful** query that returned `undefined`. This means:

1. **Caching**: React Query caches `undefined` as valid data. If the redirect doesn't complete before a re-render, or if the user navigates back, the cached `undefined` may be served.
2. **Loading states**: The query transitions from `pending` → `success` with `data: undefined`, so components render their "data loaded" state with no data instead of showing a loading spinner.
3. **Retries**: React Query won't retry because the query "succeeded."
4. **`onSuccess` callbacks**: Any configured `onSuccess` or dependent queries will fire with `undefined` input.

With `requestPromise`, none of these happen because the promise never settles — React Query stays in `pending` state (showing a loading spinner) until the page navigates away.

## Other Differences

| Aspect | `api.requestPromise()` | `sentryCellFetch()` |
| --------------- | --------------------------------------------------------------- | ---------------------------------------------------------------------------------- |
| Error type | `RequestError(method, path, preservedError, resp)` | `RequestError(method, fullUrl, new Error('Request failed'), resp)` |
| Stack trace | `preservedError` created before async call — captures call site | `new Error('Request failed')` created at throw time — captures internal stack only |
| Network failure | Silently swallowed (fetch rejection handler is `() => {}`) | Propagates as thrown error |

## Error Handler Registration

### `api.requestPromise` — `initApiClientErrorHandling()`

Pushes a single handler into `globalErrorHandlers` array. The handler covers:

- 401 + `sso-required` → `window.location.assign(loginUrl)`
- 401 + `member-disabled-over-limit` → SPA navigate to `extra.next`
- 401 (other) → set `session_expired` cookie, reload or navigate to `/auth/login/`

Handlers return `boolean` — `true` to suppress the per-request error callback.

### `sentryCellFetch` — `configureSentryCellFetch()` + `createDefaultErrorHandlers()`

Error handlers are injected via config, with named hooks:

- `onSudoRequired` → opens sudo modal, returns `Promise<ApiResponse>` (owns the retry)
- `onProjectRenamed` → calls `redirectToProject(slug)`, returns `true`
- `onAuthError` → same logic as the old handler (anon pages, SSO, member limit, session expired)
- `onError` → generic catch-all

Key structural difference: sudo/superuser handling lives **inside** `handleRequestError` in the old client (part of `Client` class), but is a pluggable `onSudoRequired` handler in `sentryCellFetch`.
69 changes: 69 additions & 0 deletions static/app/utils/api/__mocks__/sentryCellFetch.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import type {QueryFunctionContext} from '@tanstack/react-query';

import {Client} from 'sentry/api';
import type {
ApiQueryKey,
InfiniteApiQueryKey,
QueryKeyEndpointOptions,
} from 'sentry/utils/api/apiQueryKey';
import type {ApiResponse, SentryCellFetchConfig} from 'sentry/utils/api/sentryCellFetch';
import type {ParsedHeader} from 'sentry/utils/parseLinkHeader';

const mockClient = new Client();

export function configureSentryCellFetch(_config: SentryCellFetchConfig): void {
// no-op in tests
}

function buildApiResponse<T>(
json: T,
response: {getResponseHeader?: (key: string) => string | null} | undefined
): ApiResponse<T> {
const hits = response?.getResponseHeader?.('X-Hits');
const maxHits = response?.getResponseHeader?.('X-Max-Hits');
return {
headers: {
Link: response?.getResponseHeader?.('Link') ?? undefined,
'X-Hits': typeof hits === 'string' ? Number(hits) : undefined,
'X-Max-Hits': typeof maxHits === 'string' ? Number(maxHits) : undefined,
},
json,
};
}

export async function fetchWithUrl<TQueryFnData = unknown>(
url: string,
options: QueryKeyEndpointOptions = {}
): Promise<ApiResponse<TQueryFnData>> {
const [json, , response] = await mockClient.requestPromise(url, {
includeAllArgs: true,
allowAuthError: options.allowAuthError,
host: options.host,
method: options.method ?? 'GET',
data: options.data,
query: options.query,
headers: options.headers,
});

return buildApiResponse(json as TQueryFnData, response);
}

export async function sentryCellFetch<TQueryFnData = unknown>(
context: QueryFunctionContext<ApiQueryKey>
): Promise<ApiResponse<TQueryFnData>> {
const [url, options] = context.queryKey;
return fetchWithUrl(url, options);
}

export async function sentryCellFetchInfinite<TQueryFnData = unknown>(
context: QueryFunctionContext<InfiniteApiQueryKey, null | undefined | ParsedHeader>
): Promise<ApiResponse<TQueryFnData>> {
const [url, options] = context.queryKey;
return fetchWithUrl(url, {
...options,
query: {
...options?.query,
cursor: context.pageParam?.cursor ?? options?.query?.cursor,
},
});
}
Loading
Loading