From ddbba7b460fd546ed9936a1f3316d4b61ee68131 Mon Sep 17 00:00:00 2001 From: Gilles De Mey Date: Mon, 7 Jul 2025 14:56:31 +0200 Subject: [PATCH] Alerting: Add support for "alerting.notifications.routes.conflictingMatchers" (#107687) add support for "alerting.notifications.routes.conflictingMatchers" --- .../PolicyUpdateErrorAlert.tsx | 15 +----- .../alerting/unified/utils/k8s/errors.test.ts | 45 ++++++++++++++++ .../alerting/unified/utils/k8s/errors.ts | 54 ++++++++++++++++--- .../alerting/unified/utils/misc.test.ts | 6 +-- .../features/alerting/unified/utils/misc.ts | 24 ++++++--- public/locales/en-US/grafana.json | 6 ++- 6 files changed, 119 insertions(+), 31 deletions(-) create mode 100644 public/app/features/alerting/unified/utils/k8s/errors.test.ts diff --git a/public/app/features/alerting/unified/components/notification-policies/PolicyUpdateErrorAlert.tsx b/public/app/features/alerting/unified/components/notification-policies/PolicyUpdateErrorAlert.tsx index 842b4840a47..48c573c6f5e 100644 --- a/public/app/features/alerting/unified/components/notification-policies/PolicyUpdateErrorAlert.tsx +++ b/public/app/features/alerting/unified/components/notification-policies/PolicyUpdateErrorAlert.tsx @@ -4,23 +4,12 @@ import { Alert } from '@grafana/ui'; import { stringifyErrorLike } from '../../utils/misc'; export const NotificationPoliciesErrorAlert = ({ error }: { error: unknown }) => { - const title = t('alerting.policies.update-errors.title', 'Error saving notification policy'); + const title = t('alerting.policies.update-errors.title', 'Failed to add or update notification policy'); const errMessage = stringifyErrorLike(error); return ( -
- - Something went wrong when updating your notification policies. - -
-
- {errMessage || ( - - Error message: "{{ error }}" - - )} -
+
{errMessage}
Please refresh the page and try again. diff --git a/public/app/features/alerting/unified/utils/k8s/errors.test.ts b/public/app/features/alerting/unified/utils/k8s/errors.test.ts new file mode 100644 index 00000000000..ac7c03aa907 --- /dev/null +++ b/public/app/features/alerting/unified/utils/k8s/errors.test.ts @@ -0,0 +1,45 @@ +import { + ApiMachineryErrorResponse, + ERROR_ROUTES_MATCHER_CONFLICT, + getErrorMessageFromApiMachineryErrorResponse, +} from './errors'; + +describe('getErrorMessageFromCode', () => { + it(`should handle ${ERROR_ROUTES_MATCHER_CONFLICT}`, () => { + const error: ApiMachineryErrorResponse = { + status: 400, + config: { url: '' }, + data: { + kind: 'Status', + apiVersion: 'v1', + metadata: {}, + status: 'Failure', + message: 'this is ignored', + reason: 'BadRequest', + details: { + uid: 'alerting.notifications.routes.conflictingMatchers', + causes: [ + { + message: '"[foo=\\"bar\\" baz=\\"qux\\"]"', + field: 'Matchers', + }, + { + message: '"[foo2=\\"bar2\\" baz2=\\"qux2\\"]"', + field: 'Matchers', + }, + ], + }, + code: 400, + }, + }; + + expect(getErrorMessageFromApiMachineryErrorResponse(error)).toBe( + 'Cannot add or update route: matchers conflict with an external routing tree if we merged matchers \"[foo=\\\"bar\\\" baz=\\\"qux\\\"]\", \"[foo2=\\\"bar2\\\" baz2=\\\"qux2\\\"]\". This would make the route unreachable.' + ); + + delete error.data.details?.causes; + expect(getErrorMessageFromApiMachineryErrorResponse(error)).toBe( + 'Cannot add or update route: matchers conflict with an external routing tree if we merged matchers . This would make the route unreachable.' + ); + }); +}); diff --git a/public/app/features/alerting/unified/utils/k8s/errors.ts b/public/app/features/alerting/unified/utils/k8s/errors.ts index 3d315471321..0bb486710a1 100644 --- a/public/app/features/alerting/unified/utils/k8s/errors.ts +++ b/public/app/features/alerting/unified/utils/k8s/errors.ts @@ -3,19 +3,56 @@ import { get } from 'lodash'; import { t } from '@grafana/i18n'; import { FetchError, isFetchError } from '@grafana/runtime'; -export type SupportedErrors = 'alerting.notifications.conflict' | string; +import { getErrorCode } from '../misc'; -export const ERROR_NEWER_CONFIGURATION = 'alerting.notifications.conflict'; +export const ERROR_NEWER_CONFIGURATION = 'alerting.notifications.conflict' as const; +export const ERROR_ROUTES_MATCHER_CONFLICT = 'alerting.notifications.routes.conflictingMatchers' as const; -/** This function gives us the opportunity to translate or transform error codes that are returned from the Kubernetes APIs */ +export type ApiMachineryErrorResponse = FetchError; + +// these are known error IDs, used by both Kubernetes API and the front-end (using error `cause`). +export type KnownErrorCodes = typeof ERROR_NEWER_CONFIGURATION; +// Kubernetes API Machinery errors are a superset of supported errors codes. +export type KnownMachineryErrorCodes = KnownErrorCodes | typeof ERROR_ROUTES_MATCHER_CONFLICT; + +/** + * This function gives us the opportunity to translate or transform error codes that are returned from the Kubernetes APIs + */ +export function getErrorMessageFromApiMachineryErrorResponse(error: ApiMachineryErrorResponse): string | undefined { + const code = getErrorCode(error); + if (!code) { + return error.data.message; + } + + const errorMessageMap: Record = { + [ERROR_NEWER_CONFIGURATION]: getErrorMessageFromCode(code), + [ERROR_ROUTES_MATCHER_CONFLICT]: t( + 'alerting.policies.update-errors.routes.conflictingMatchers', + 'Cannot add or update route: matchers conflict with an external routing tree if we merged matchers {{-matchers}}. This would make the route unreachable.', + { + matchers: + error.data.details?.causes?.map((cause) => cause.message).join(', ') ?? + t('alerting.policies.update-errors.routes.unknownMatchers', ''), + } + ), + }; + + // @ts-expect-error this allows the typechecker to warn us about forgetting to handle some KnownMachineryErrors and still return "undefined" for unknown codes; + return errorMessageMap[code]; +} + +/** + * This function gives us the opportunity to translate or transform error codes + */ export function getErrorMessageFromCode(code: string): string | undefined { - const errorMessageMap: Record = { + const errorMessageMap: Record = { [ERROR_NEWER_CONFIGURATION]: t( 'alerting.policies.update-errors.conflict', 'The notification policy tree has been updated by another user.' ), }; + // @ts-expect-error this allows the typechecker to warn us about forgetting to handle some KnownErrors and still return "undefined" for unknown codes; return errorMessageMap[code]; } @@ -29,7 +66,12 @@ export type ApiMachineryError = { group?: string; kind?: string; retryAfterSeconds?: number; - causes?: []; // @TODO type this, see apimachinery@v0.31.1/pkg/apis/meta/v1/types.go + // https://github.com/kubernetes/apimachinery/blob/v0.33.2/pkg/apis/meta/v1/types.go#L1020-L1040 + causes?: Array<{ + message?: string; + field?: string; + reason?: string; + }>; }; status: 'Failure'; metadata?: Record; @@ -37,6 +79,6 @@ export type ApiMachineryError = { reason: string; }; -export function isApiMachineryError(error: unknown): error is FetchError { +export function isApiMachineryError(error: unknown): error is ApiMachineryErrorResponse { return isFetchError(error) && get(error.data, 'kind') === 'Status' && get(error.data, 'status') === 'Failure'; } diff --git a/public/app/features/alerting/unified/utils/misc.test.ts b/public/app/features/alerting/unified/utils/misc.test.ts index 8dae9443827..8916603f2ee 100644 --- a/public/app/features/alerting/unified/utils/misc.test.ts +++ b/public/app/features/alerting/unified/utils/misc.test.ts @@ -125,12 +125,12 @@ describe('create links', () => { describe('stringifyErrorLike', () => { it('should stringify error with cause', () => { - const error = new Error('Something went strong', { cause: new Error('database did not respond') }); - expect(stringifyErrorLike(error)).toBe('Something went strong, cause: database did not respond'); + const error = new Error('Something went wrong', { cause: new Error('database did not respond') }); + expect(stringifyErrorLike(error)).toBe('Something went wrong, cause: database did not respond'); }); it('should stringify error with cause being a code', () => { - const error = new Error('Something went strong', { cause: ERROR_NEWER_CONFIGURATION }); + const error = new Error('Something went wrong', { cause: ERROR_NEWER_CONFIGURATION }); expect(stringifyErrorLike(error)).toBe(getErrorMessageFromCode(ERROR_NEWER_CONFIGURATION)); }); diff --git a/public/app/features/alerting/unified/utils/misc.ts b/public/app/features/alerting/unified/utils/misc.ts index 561e5be930d..4501be47b69 100644 --- a/public/app/features/alerting/unified/utils/misc.ts +++ b/public/app/features/alerting/unified/utils/misc.ts @@ -1,4 +1,4 @@ -import { sortBy } from 'lodash'; +import { isString, sortBy } from 'lodash'; import { Labels, UrlQueryMap } from '@grafana/data'; import { GrafanaEdition } from '@grafana/data/internal'; @@ -34,7 +34,12 @@ import { import { ALERTMANAGER_NAME_QUERY_KEY } from './constants'; import { getRulesSourceName } from './datasource'; -import { SupportedErrors, getErrorMessageFromCode, isApiMachineryError } from './k8s/errors'; +import { + KnownErrorCodes, + getErrorMessageFromApiMachineryErrorResponse, + getErrorMessageFromCode, + isApiMachineryError, +} from './k8s/errors'; import { getMatcherQueryParams } from './matchers'; import { rulesNav } from './navigation'; import * as ruleId from './rule-id'; @@ -282,15 +287,20 @@ export function isErrorLike(error: unknown): error is Error { return Boolean(error && typeof error === 'object' && 'message' in error); } -export function getErrorCode(error: Error): unknown { +export function getErrorCode(error: unknown): string | undefined { if (isApiMachineryError(error) && error.data.details) { return error.data.details.uid; } - return error.cause; + + if (isErrorLike(error) && isString(error.cause)) { + return error.cause; + } + + return; } /* this function will check if the error passed as the first argument contains an error code */ -export function isErrorMatchingCode(error: Error | undefined, code: SupportedErrors): boolean { +export function isErrorMatchingCode(error: Error | undefined, code: KnownErrorCodes): boolean { if (!error) { return false; } @@ -301,8 +311,8 @@ export function isErrorMatchingCode(error: Error | undefined, code: SupportedErr export function stringifyErrorLike(error: unknown): string { const fetchError = isFetchError(error); if (fetchError) { - if (isApiMachineryError(error) && error.data.details) { - const message = getErrorMessageFromCode(error.data.details.uid); + if (isApiMachineryError(error)) { + const message = getErrorMessageFromApiMachineryErrorResponse(error); if (message) { return message; } diff --git a/public/locales/en-US/grafana.json b/public/locales/en-US/grafana.json index 226dc40018d..fc392dece19 100644 --- a/public/locales/en-US/grafana.json +++ b/public/locales/en-US/grafana.json @@ -2095,9 +2095,11 @@ "update-errors": { "conflict": "The notification policy tree has been updated by another user.", "error-code": "Error message: \"{{error}}\"", - "fallback": "Something went wrong when updating your notification policies.", + "routes": { + "conflictingMatchers": "Cannot add or update route: matchers conflict with an external routing tree if we merged matchers {{-matchers}}. This would make the route unreachable." + }, "suffix": "Please refresh the page and try again.", - "title": "Error saving notification policy" + "title": "Failed to add or update notification policy" } }, "policy": {