fix(integrations): show error instead of infinite spinner on sentry app external install page#112432
fix(integrations): show error instead of infinite spinner on sentry app external install page#112432sentry-junior[bot] wants to merge 3 commits intomasterfrom
Conversation
…pp external install page When the /sentry-apps/:slug/ API returns a non-2xx response (e.g. 403 for unpublished apps where the user is not in the owner org), useApiQuery sets sentryAppLoading=false but sentryApp=undefined. The existing guard: if (sentryAppLoading || orgsLoading || !sentryApp) falls through to !sentryApp=true and renders <LoadingIndicator /> forever with no feedback to the user. Fix: destructure isError from useApiQuery and track orgsError state, then render <LoadingError> with a descriptive message for each error case — following the pattern in sentryAppDetailedView.tsx. Also adds a test covering the 403 case.
| if (sentryAppLoading || orgsLoading) { | ||
| return <LoadingIndicator />; | ||
| } |
There was a problem hiding this comment.
Bug: The loading check (sentryAppLoading || orgsLoading) can hide an error from one request if the other is still pending, delaying the error message display.
Severity: LOW
Suggested Fix
Prioritize displaying errors over the loading state. Move the error checks for sentryAppError and orgsError before the combined loading check for sentryAppLoading || orgsLoading. This ensures that if any request fails, its corresponding error message is shown immediately, regardless of the status of other pending requests.
Prompt for AI Agent
Review the code at the location below. A potential bug has been identified by an AI
agent.
Verify if this is a real issue. If it is, propose a fix; if not, explain why it's not
valid.
Location: static/app/views/sentryAppExternalInstallation/index.tsx#L201-L203
Potential issue: A race condition exists in the loading and error handling logic. If one
of two parallel data requests (for the Sentry app or organizations) fails while the
other is still in progress, the UI will continue to show a loading indicator. The
condition `if (sentryAppLoading || orgsLoading)` prevents the error handling logic from
being reached until both requests have completed. This results in a delayed error
message, where the user sees a loading spinner instead of an immediate error, even after
one of the underlying API calls has already failed.
Did we get this right? 👍 / 👎 to inform future reviews.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 4723c2b. Configure here.
| data: sentryApp, | ||
| isPending: sentryAppLoading, | ||
| isError: sentryAppError, | ||
| } = useApiQuery<SentryApp>( |
There was a problem hiding this comment.
Missing retry: false causes delayed error display
Medium Severity
The useApiQuery call is missing retry: false, unlike the reference pattern in sentryAppDetailedView.tsx that this PR claims to follow. React Query's default retries 3 times with exponential backoff. During retries, isPending stays true, so the loading spinner shows for ~7+ seconds before isError becomes true and the error message appears. For deterministic errors like 403, retrying is futile and just delays the user-facing error.
Reviewed by Cursor Bugbot for commit 4723c2b. Configure here.


Problem
When visiting the external install page for an unpublished sentry app where the user is not in the owner org, the API returns a 403:
useApiQuerycorrectly marks the request as errored but the existing guard:hits
!sentryApp === trueand renders a<LoadingIndicator />forever with no feedback to the user. Same issue applies to any other non-2xx response (404, 500, etc.).Fix
isErrorfromuseApiQueryfor the sentry app fetchorgsErrorstate for the orgs fetch (was silently swallowed before)<LoadingIndicator />while pending; render<LoadingError />for each error casesentryAppDetailedView.tsxTest
Added a test case covering the 403 scenario.