Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
4 changes: 3 additions & 1 deletion static/app/router/routes.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1658,7 +1658,9 @@ function buildRoutes(): RouteObject[] {
},
{
path: ':alertId/',
component: make(() => import('sentry/views/alerts/incidentRedirect')),
component: make(
() => import('sentry/views/alerts/workflowEngineRedirectWrappers/incident')
),
},
{
path: ':projectId/',
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import {lazy} from 'react';

import {withOpenPeriodRedirect} from 'sentry/views/alerts/workflowEngineRedirects';

const AlertWizardProjectProvider = lazy(
() => import('sentry/views/alerts/incidentRedirect')
);

export default withOpenPeriodRedirect(AlertWizardProjectProvider);
133 changes: 129 additions & 4 deletions static/app/views/alerts/workflowEngineRedirects.tsx
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import LoadingIndicator from 'sentry/components/loadingIndicator';
import Redirect from 'sentry/components/redirect';
import {useApiQuery} from 'sentry/utils/queryClient';
import {useLocation} from 'sentry/utils/useLocation';
import useOrganization from 'sentry/utils/useOrganization';
import {useParams} from 'sentry/utils/useParams';
import {useUser} from 'sentry/utils/useUser';
Expand All @@ -26,6 +27,13 @@ interface AlertRuleDetector {
ruleId: string | null;
}

interface IncidentGroupOpenPeriod {
groupId: string;
incidentId: string | null;
incidentIdentifier: string;
openPeriodId: string;
}

/**
* HoC that wraps a component to handle workflow engine
* redirects for issue alert rules. Fetches workflow data if needed and
Expand Down Expand Up @@ -137,10 +145,86 @@ export const withAutomationEditRedirect = <P extends Record<string, any>>(

export const withDetectorDetailsRedirect = <P extends Record<string, any>>(
Component: React.ComponentType<P>
) =>
withAlertRuleRedirect(Component, (detectorId, orgSlug) =>
makeMonitorDetailsPathname(orgSlug, detectorId)
);
) => {
return function WorkflowEngineRedirectWrapper(props: P) {
const user = useUser();
const organization = useOrganization();
const {ruleId, detectorId} = useParams();
const location = useLocation();
const alertId = location.query.alert as string | undefined;

const shouldRedirect =
!user.isStaff && organization.features.includes('workflow-engine-ui');

// Check for incident open period if alertId is present
const {data: incidentGroupOpenPeriod, isPending: isOpenPeriodPending} =
useApiQuery<IncidentGroupOpenPeriod>(
[
`/organizations/${organization.slug}/incident-groupopenperiod/`,
{query: {incident_identifier: alertId}},
],
{
staleTime: 0,
enabled: shouldRedirect && !!alertId,
retry: false,
}
);

// Check for detector if no alertId
const {data: alertRuleDetector, isPending: isDetectorPending} =
useApiQuery<AlertRuleDetector>(
[
`/organizations/${organization.slug}/alert-rule-detector/`,
{query: {alert_rule_id: ruleId}},
],
{
staleTime: 0,
enabled: shouldRedirect && !!ruleId && !detectorId && !alertId,
retry: false,
}
);

if (shouldRedirect) {
// If alertId is provided, redirect to metric issue
if (alertId) {
if (isOpenPeriodPending) {
return <LoadingIndicator />;
}
if (incidentGroupOpenPeriod) {
return (
<Redirect
to={`/organizations/${organization.slug}/issues/${incidentGroupOpenPeriod.groupId}`}
/>
);
}
}

// If detectorId is provided, redirect to monitor details
if (detectorId) {
return (
<Redirect to={makeMonitorDetailsPathname(organization.slug, detectorId)} />
);
}

// If alertRuleId is provided, fetch detector and redirect
if (isDetectorPending) {
return <LoadingIndicator />;
}
if (alertRuleDetector) {
return (
<Redirect
to={makeMonitorDetailsPathname(
organization.slug,
alertRuleDetector.detectorId
)}
/>
);
}
}

return <Component {...(props as any)} />;
};
};

export const withDetectorEditRedirect = <P extends Record<string, any>>(
Component: React.ComponentType<P>
Expand Down Expand Up @@ -183,3 +267,44 @@ export function withDetectorCreateRedirect<P extends Record<string, any>>(
return <Component {...(props as any)} />;
};
}

export function withOpenPeriodRedirect<P extends Record<string, any>>(
Component: React.ComponentType<P>
) {
return function OpenPeriodRedirectWrapper(props: P) {
const user = useUser();
const organization = useOrganization();
const {alertId} = useParams();

const shouldRedirect =
!user.isStaff && organization.features.includes('workflow-engine-ui');

const {data: incidentGroupOpenPeriod, isPending} =
useApiQuery<IncidentGroupOpenPeriod>(
[
`/organizations/${organization.slug}/incident-groupopenperiod/`,
{query: {incident_identifier: alertId}},
],
{
staleTime: 0,
enabled: shouldRedirect && !!alertId,
Comment on lines +283 to +290
Copy link

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

retry: false,
}
);

if (shouldRedirect) {
if (isPending) {
return <LoadingIndicator />;
}
if (incidentGroupOpenPeriod) {
return (
<Redirect
to={`/organizations/${organization.slug}/issues/${incidentGroupOpenPeriod.groupId}`}
/>
);
}
}

return <Component {...(props as any)} />;
};
}
Loading