-
-
Notifications
You must be signed in to change notification settings - Fork 4.5k
feat(aci): redirect incidents to metric issue details #104001
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
| useApiQuery<IncidentGroupOpenPeriod>( | ||
| [ | ||
| `/organizations/${organization.slug}/incident-groupopenperiod/`, | ||
| {query: {incident_identifier: alertId}}, | ||
| ], | ||
| { | ||
| staleTime: 0, | ||
| enabled: shouldRedirect && !!alertId, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Bug: withOpenPeriodRedirect lacks error handling for useApiQuery failures, causing silent fallbacks and cascading issues when /incident-groupopenperiod/ returns errors.
Severity: CRITICAL | Confidence: High
🔍 Detailed Analysis
The withOpenPeriodRedirect function uses useApiQuery to fetch data from /incident-groupopenperiod/ but lacks error handling for API failures. When the API request fails (e.g., returns a 404, 400, or 5xx error), the isError state from useApiQuery is not checked. This causes the component to silently fall through and render the wrapped component (AlertWizardProjectProvider), leading to a cascading failure where the user experiences indefinite loading or repeated failures from the old incidentRedirect component.
💡 Suggested Fix
Add an if (isError) check within withOpenPeriodRedirect to display an appropriate error message or component (e.g., <LoadingError />) when useApiQuery fails to fetch data from /incident-groupopenperiod/.
🤖 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/alerts/workflowEngineRedirects.tsx#L283-L290
Potential issue: The `withOpenPeriodRedirect` function uses `useApiQuery` to fetch data
from `/incident-groupopenperiod/` but lacks error handling for API failures. When the
API request fails (e.g., returns a 404, 400, or 5xx error), the `isError` state from
`useApiQuery` is not checked. This causes the component to silently fall through and
render the wrapped component (`AlertWizardProjectProvider`), leading to a cascading
failure where the user experiences indefinite loading or repeated failures from the old
`incidentRedirect` component.
Did we get this right? 👍 / 👎 to inform future reviews.
Reference ID: 3622188
redirects metric alert incidents to the appropriate metric issue.
note we don't currently support passing a specific open period id to the UI, but we can easily add this to the redirect logic later.