fix(planned-downtime): notification breaking the page due to invalid description

frontend/src/container/PlannedDowntime/PlannedDowntimeForm.tsx

@@ -175,7 +175,10 @@ export function PlannedDowntimeForm(
 				} else {
 					notifications.error({
 						message: 'Error',
-						description: response.error || 'unexpected_error',
+						description:
+							typeof response.error === 'string'
+								? response.error
+								: response.error?.message || 'unexpected_error',
 					});
 				}
 			} catch (e) {

frontend/src/container/PlannedDowntime/PlannedDowntimeList.tsx

@@ -359,7 +359,10 @@ export function PlannedDowntimeList({
 
 	useEffect(() => {
 		if (downtimeSchedules.isError) {
-			notifications.error(downtimeSchedules.error);
+			notifications.error({
+				message:
+					downtimeSchedules.error?.message?.toString() || 'Something went wrong',
+			});
 		}
 	}, [downtimeSchedules.error, downtimeSchedules.isError, notifications]);
 

frontend/src/container/PlannedDowntime/PlannedDowntimeutils.ts

@@ -137,7 +137,10 @@ export const deleteDowntimeHandler = ({
 
 export const createEditDowntimeSchedule = async (
 	props: DowntimeScheduleUpdatePayload,
-): Promise<SuccessResponse<PayloadProps> | ErrorResponse> => {
+): Promise<
+	| SuccessResponse<PayloadProps>
+	| ErrorResponse<{ code: string; message: string } | string>
+> => {
 	if (props.id) {
 		return updateDowntimeSchedule({ ...props });
 	}

frontend/src/hooks/__tests__/useNotifications.test.tsx

@@ -0,0 +1,334 @@
+/* eslint-disable @typescript-eslint/explicit-function-return-type */
+/* eslint-disable @typescript-eslint/no-explicit-any */
+
+import type { ReactElement, ReactNode } from 'react';
+import { createElement } from 'react';
+import { act, renderHook } from '@testing-library/react';
+
+import { NotificationProvider, useNotifications } from '../useNotifications';
+
+const mockSuccess = jest.fn();
+const mockError = jest.fn();
+const mockInfo = jest.fn();
+const mockWarning = jest.fn();
+const mockOpen = jest.fn();
+const mockDestroy = jest.fn();
+
+jest.mock('antd', () => ({
+	notification: {
+		useNotification: () => [
+			{
+				success: mockSuccess,
+				error: mockError,
+				info: mockInfo,
+				warning: mockWarning,
+				open: mockOpen,
+				destroy: mockDestroy,
+			},
+			createElement('div', { 'data-testid': 'notification-holder' }),
+		],
+	},
+}));
+
+const mockSetExtra = jest.fn();
+const mockCaptureMessage = jest.fn();
+
+jest.mock('@sentry/react', () => ({
+	setExtra: (key: string, value: unknown) => mockSetExtra(key, value),
+	captureMessage: (msg: string) => mockCaptureMessage(msg),
+}));
+
+describe('useNotifications', () => {
+	beforeEach(() => {
+		jest.clearAllMocks();
+	});
+
+	describe('without provider', () => {
+		it('returns default notification API and does not throw', () => {
+			const { result } = renderHook(() => useNotifications());
+
+			expect(result.current.notifications).toBeDefined();
+			expect(typeof result.current.notifications.success).toBe('function');
+			expect(typeof result.current.notifications.error).toBe('function');
+			expect(typeof result.current.notifications.info).toBe('function');
+			expect(typeof result.current.notifications.warning).toBe('function');
+			expect(typeof result.current.notifications.open).toBe('function');
+			expect(typeof result.current.notifications.destroy).toBe('function');
+		});
+
+		it('no-op methods do not throw when called', () => {
+			const { result } = renderHook(() => useNotifications());
+
+			expect(() => {
+				result.current.notifications.success({ message: 'Ok' });
+				result.current.notifications.error({ message: 'Fail' });
+				result.current.notifications.info({ message: 'Info' });
+				result.current.notifications.warning({ message: 'Warn' });
+				result.current.notifications.open({ message: 'Open' });
+				result.current.notifications.destroy();
+				result.current.notifications.destroy('key');
+			}).not.toThrow();
+		});
+	});
+
+	describe('with NotificationProvider', () => {
+		const wrapper = ({ children }: { children: ReactNode }) =>
+			createElement(NotificationProvider, null, children as ReactElement);
+
+		it('returns notification API that forwards valid payloads', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.success({
+					message: 'Saved',
+					description: 'Your changes were saved.',
+				});
+			});
+
+			expect(mockSuccess).toHaveBeenCalledTimes(1);
+			expect(mockSuccess).toHaveBeenCalledWith({
+				message: 'Saved',
+				description: 'Your changes were saved.',
+			});
+		});
+
+		it('forwards valid payloads for error, info, warning, open', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.error({ message: 'Error msg' });
+			});
+			expect(mockError).toHaveBeenCalledWith({ message: 'Error msg' });
+
+			act(() => {
+				result.current.notifications.info({ message: 'Info msg' });
+			});
+			expect(mockInfo).toHaveBeenCalledWith({ message: 'Info msg' });
+
+			act(() => {
+				result.current.notifications.warning({ message: 'Warn msg' });
+			});
+			expect(mockWarning).toHaveBeenCalledWith({ message: 'Warn msg' });
+
+			act(() => {
+				result.current.notifications.open({ message: 'Open msg' });
+			});
+			expect(mockOpen).toHaveBeenCalledWith({ message: 'Open msg' });
+		});
+
+		it('forwards destroy to the underlying API', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.destroy('some-key');
+			});
+			expect(mockDestroy).toHaveBeenCalledWith('some-key');
+
+			act(() => {
+				result.current.notifications.destroy();
+			});
+			expect(mockDestroy).toHaveBeenCalledWith(undefined);
+		});
+
+		it('accepts React element as message and forwards it', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+			const element = createElement('span', null, 'Custom message');
+
+			act(() => {
+				result.current.notifications.success({ message: element });
+			});
+
+			expect(mockSuccess).toHaveBeenCalledWith({ message: element });
+			expect(mockSetExtra).not.toHaveBeenCalled();
+			expect(mockCaptureMessage).not.toHaveBeenCalled();
+		});
+
+		it('accepts empty object and does not break', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+			const emptyPayload = {} as Parameters<
+				ReturnType<typeof useNotifications>['notifications']['success']
+			>[0];
+
+			act(() => {
+				result.current.notifications.success(emptyPayload);
+			});
+
+			expect(mockSuccess).toHaveBeenCalledWith(emptyPayload);
+			expect(mockSetExtra).not.toHaveBeenCalled();
+			expect(mockCaptureMessage).not.toHaveBeenCalled();
+		});
+	});
+
+	describe('invalid payloads do not break the app', () => {
+		const wrapper = ({ children }: { children: ReactNode }) =>
+			createElement(NotificationProvider, null, children as ReactElement);
+
+		it('rejects non-object args and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.success(null as any);
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', null);
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockSuccess).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('rejects undefined and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.error(undefined as any);
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', undefined);
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockError).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('rejects primitive args (string, number) and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.info('just a string' as any);
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith(
+				'notificationArgs',
+				'just a string',
+			);
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockInfo).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+
+			jest.clearAllMocks();
+
+			act(() => {
+				result.current.notifications.warning(42 as any);
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', 42);
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockWarning).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('rejects message as number and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.success({
+					message: 123 as any,
+					description: 'Valid description',
+				});
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', {
+				message: 123,
+				description: 'Valid description',
+			});
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockSuccess).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('rejects description as number and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.error({
+					message: 'Valid message',
+					description: 999 as any,
+				});
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', {
+				message: 'Valid message',
+				description: 999,
+			});
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockError).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('rejects icon as number and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.open({
+					message: 'Valid',
+					icon: 0 as any,
+				});
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', {
+				message: 'Valid',
+				icon: 0,
+			});
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockOpen).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('rejects message as object (non-React element) and shows fallback', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			act(() => {
+				result.current.notifications.success({
+					message: { foo: 'bar' } as any,
+				});
+			});
+			expect(mockSetExtra).toHaveBeenCalledWith('notificationArgs', {
+				message: { foo: 'bar' },
+			});
+			expect(mockCaptureMessage).toHaveBeenCalledWith(
+				'invalid_args_for_notification',
+			);
+			expect(mockSuccess).toHaveBeenCalledWith({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		});
+
+		it('invalid payloads never throw', () => {
+			const { result } = renderHook(() => useNotifications(), { wrapper });
+
+			expect(() => {
+				act(() => {
+					result.current.notifications.success(null as any);
+					result.current.notifications.error(undefined as any);
+					result.current.notifications.info(123 as any);
+					result.current.notifications.warning({ message: 0 as any });
+					result.current.notifications.open({
+						message: 'x',
+						description: [],
+					} as any);
+				});
+			}).not.toThrow();
+		});
+	});
+});

frontend/src/hooks/useNotifications.tsx

@@ -1,30 +1,75 @@
-import {
-	// eslint-disable-next-line no-restricted-imports
-	createContext,
-	// eslint-disable-next-line no-restricted-imports
-	useContext,
-	useMemo,
-} from 'react';
+import { isValidElement, useEffect } from 'react';
+import * as Sentry from '@sentry/react';
 import { notification } from 'antd';
 import type { NotificationInstance } from 'antd/es/notification/interface';
+import { create } from 'zustand';
+import { useShallow } from 'zustand/react/shallow';
 
 type Notification = {
 	notifications: NotificationInstance;
 };
 
-const defaultNotification: Notification = {
-	notifications: {
-		success: (): void => {},
-		error: (): void => {},
-		info: (): void => {},
-		warning: (): void => {},
-		open: (): void => {},
-		destroy: (): void => {},
-	},
+const defaultNotificationInstance: NotificationInstance = {
+	success: (): void => {},
+	error: (): void => {},
+	info: (): void => {},
+	warning: (): void => {},
+	open: (): void => {},
+	destroy: (): void => {},
 };
 
-export const NotificationContext = createContext<Notification>(
-	defaultNotification,
+type NotificationFn = NotificationInstance['success'];
+
+const guardNotificationPayload = (
+	notificationApi: NotificationInstance,
+	method: 'success' | 'error' | 'info' | 'warning' | 'open',
+): NotificationFn => {
+	return (notificationArgs): void => {
+		if (
+			!notificationArgs ||
+			typeof notificationArgs !== 'object' ||
+			(notificationArgs?.message !== undefined &&
+				typeof notificationArgs.message !== 'string' &&
+				!isValidElement(notificationArgs.message)) ||
+			(notificationArgs?.icon !== undefined &&
+				typeof notificationArgs.icon !== 'string' &&
+				!isValidElement(notificationArgs.icon)) ||
+			(notificationArgs?.description !== undefined &&
+				typeof notificationArgs.description !== 'string' &&
+				!isValidElement(notificationArgs.description))
+		) {
+			Sentry.setExtra('notificationArgs', notificationArgs);
+			Sentry.captureMessage('invalid_args_for_notification');
+
+			return notificationApi[method]({
+				message: 'Error',
+				description: 'Something went wrong',
+			});
+		}
+
+		return notificationApi[method](notificationArgs);
+	};
+};
+
+type NotificationStore = Notification & {
+	setNotifications: (notifications: NotificationInstance) => void;
+};
+
+export const useNotificationStore = create<NotificationStore>(
+	(set): NotificationStore => ({
+		notifications: defaultNotificationInstance,
+		setNotifications: (notifications: NotificationInstance): void =>
+			set({
+				notifications: {
+					success: guardNotificationPayload(notifications, 'success'),
+					error: guardNotificationPayload(notifications, 'error'),
+					info: guardNotificationPayload(notifications, 'info'),
+					warning: guardNotificationPayload(notifications, 'warning'),
+					open: guardNotificationPayload(notifications, 'open'),
+					destroy: (key?: string): void => notifications.destroy(key),
+				},
+			}),
+	}),
 );
 
 export function NotificationProvider({
@@ -33,17 +78,21 @@ export function NotificationProvider({
 	children: JSX.Element;
 }): JSX.Element {
 	const [notificationApi, NotificationElement] = notification.useNotification();
-	const notifications = useMemo(() => ({ notifications: notificationApi }), [
-		notificationApi,
-	]);
+	const setNotifications = useNotificationStore((s) => s.setNotifications);
+
+	useEffect(() => {
+		setNotifications(notificationApi);
+	}, [notificationApi, setNotifications]);
 
 	return (
-		<NotificationContext.Provider value={notifications}>
+		<>
 			{NotificationElement}
 			{children}
-		</NotificationContext.Provider>
+		</>
 	);
 }
 
 export const useNotifications = (): Notification =>
-	useContext(NotificationContext);
+	useNotificationStore(
+		useShallow((state) => ({ notifications: state.notifications })),
+	);

frontend/src/types/api/index.ts

@@ -3,10 +3,10 @@ import { ErrorStatusCode, SuccessStatusCode } from 'types/common';
 
 export type ApiResponse<T> = { data: T };
 
-export interface ErrorResponse {
+export interface ErrorResponse<ErrorObject = string> {
 	statusCode: ErrorStatusCode;
 	payload: null;
-	error: string;
+	error: ErrorObject;
 	message: string | null;
 	body?: string | null;
 }

pkg/querier/postprocess.go

@@ -185,6 +185,22 @@ func postProcessMetricQuery(
 	query qbtypes.QueryBuilderQuery[qbtypes.MetricAggregation],
 	req *qbtypes.QueryRangeRequest,
 ) *qbtypes.Result {
+
+	config := query.Aggregations[0]
+	spaceAggOrderBy := fmt.Sprintf("%s(%s)", config.SpaceAggregation.StringValue(), config.MetricName)
+	timeAggOrderBy := fmt.Sprintf("%s(%s)", config.TimeAggregation.StringValue(), config.MetricName)
+	timeSpaceAggOrderBy := fmt.Sprintf("%s(%s(%s))", config.SpaceAggregation.StringValue(), config.TimeAggregation.StringValue(), config.MetricName)
+
+	for idx := range query.Order {
+		if query.Order[idx].Key.Name == spaceAggOrderBy ||
+			query.Order[idx].Key.Name == timeAggOrderBy ||
+			query.Order[idx].Key.Name == timeSpaceAggOrderBy {
+			query.Order[idx].Key.Name = qbtypes.DefaultOrderByKey
+		}
+	}
+
+	result = q.applySeriesLimit(result, query.Limit, query.Order)
+
 	if len(query.Functions) > 0 {
 		step := query.StepInterval.Duration.Milliseconds()
 		functions := q.prepareFillZeroArgsWithStep(query.Functions, req, step)

pkg/querybuilder/fallback_expr.go

@@ -132,14 +132,6 @@ func GroupByKeys(keys []qbtypes.GroupByKey) []string {
 	return k
 }
 
-func OrderByKeys(keys []qbtypes.OrderBy) []string {
-	k := []string{}
-	for _, key := range keys {
-		k = append(k, "`"+key.Key.Name+"`")
-	}
-	return k
-}
-
 func FormatValueForContains(value any) string {
 	if value == nil {
 		return ""

pkg/telemetrymetrics/statement_builder.go

@@ -27,10 +27,6 @@ const (
 	OthersMultiTemporality = `IF(LOWER(temporality) LIKE LOWER('delta'), %s, %s) AS per_series_value`
 )
 
-const (
-	FINAL_VALUE_VARNAME = "value"
-)
-
 type MetricQueryStatementBuilder struct {
 	logger        *slog.Logger
 	metadataStore telemetrytypes.MetadataStore
@@ -242,7 +238,7 @@ func (b *MetricQueryStatementBuilder) buildTemporalAggDeltaFastPath(
 		aggCol = fmt.Sprintf("quantilesDDMerge(0.01, %f)(sketch)[1]", query.Aggregations[0].SpaceAggregation.Percentile())
 	}
 
-	sb.SelectMore(fmt.Sprintf("%s AS %s", aggCol, FINAL_VALUE_VARNAME))
+	sb.SelectMore(fmt.Sprintf("%s AS value", aggCol))
 
 	tbl := WhichSamplesTableToUse(start, end, query.Aggregations[0].Type, query.Aggregations[0].TimeAggregation, query.Aggregations[0].TableHints)
 	sb.From(fmt.Sprintf("%s.%s AS points", DBName, tbl))
@@ -530,7 +526,7 @@ func (b *MetricQueryStatementBuilder) buildSpatialAggregationCTE(
 	for _, g := range query.GroupBy {
 		sb.SelectMore(fmt.Sprintf("`%s`", g.TelemetryFieldKey.Name))
 	}
-	sb.SelectMore(fmt.Sprintf("%s(per_series_value) AS %s", query.Aggregations[0].SpaceAggregation.StringValue(), FINAL_VALUE_VARNAME))
+	sb.SelectMore(fmt.Sprintf("%s(per_series_value) AS value", query.Aggregations[0].SpaceAggregation.StringValue()))
 	sb.From("__temporal_aggregation_cte")
 	sb.Where(sb.EQ("isNaN(per_series_value)", 0))
 	if query.Aggregations[0].ValueFilter != nil {
@@ -567,10 +563,8 @@ func (b *MetricQueryStatementBuilder) BuildFinalSelect(
 			sb.SelectMore(fmt.Sprintf("`%s`", g.TelemetryFieldKey.Name))
 		}
 		sb.SelectMore(fmt.Sprintf(
-			"histogramQuantile(arrayMap(x -> toFloat64(x), groupArray(le)), groupArray(%s), %.3f) AS %s",
-			FINAL_VALUE_VARNAME,
+			"histogramQuantile(arrayMap(x -> toFloat64(x), groupArray(le)), groupArray(value), %.3f) AS value",
 			quantile,
-			FINAL_VALUE_VARNAME,
 		))
 		sb.From("__spatial_aggregation_cte")
 		sb.GroupBy(querybuilder.GroupByKeys(query.GroupBy)...)
@@ -613,30 +607,11 @@ func (b *MetricQueryStatementBuilder) BuildFinalSelect(
 			sb.Where(rewrittenExpr)
 		}
 	}
-	if len(query.Order) > 0 {
-		config := query.Aggregations[0]
-		spaceAggOrderBy := fmt.Sprintf("%s(%s)", config.SpaceAggregation.StringValue(), config.MetricName)
-		timeAggOrderBy := fmt.Sprintf("%s(%s)", config.TimeAggregation.StringValue(), config.MetricName)
-		timeSpaceAggOrderBy := fmt.Sprintf("%s(%s(%s))", config.SpaceAggregation.StringValue(), config.TimeAggregation.StringValue(), config.MetricName)
-
-		for idx := range query.Order {
-			if query.Order[idx].Key.Name == spaceAggOrderBy ||
-				query.Order[idx].Key.Name == timeAggOrderBy ||
-				query.Order[idx].Key.Name == timeSpaceAggOrderBy {
-				query.Order[idx].Key.Name = FINAL_VALUE_VARNAME
-			}
-		}
-		sb.OrderBy(querybuilder.OrderByKeys(query.Order)...)
-	} else {
-		sb.OrderBy(querybuilder.GroupByKeys(query.GroupBy)...)
-	}
+	sb.OrderBy(querybuilder.GroupByKeys(query.GroupBy)...)
 	sb.OrderBy("ts")
 	if metricType == metrictypes.HistogramType && spaceAgg == metrictypes.SpaceAggregationCount && query.Aggregations[0].ComparisonSpaceAggregationParam == nil {
 		sb.OrderBy("toFloat64(le)")
 	}
-	if query.Limit > 0 {
-		sb.Limit(query.Limit)
-	}
 
 	q, a := sb.BuildWithFlavor(sqlbuilder.ClickHouse)
 	return &qbtypes.Statement{Query: combined + q, Args: append(args, a...)}, nil