Skip to content

Commit 623cdf7

Browse files
khandrew-redhat“khandrew-redhat”
andauthored
Improved UI toasting of API errors (#293)
Toast messages now do the following: 1. Account for different json structures when extracting message 2. Do not allow the pages to hand when error occurs 3. Fade in and out, lasting no longer than five seconds when displayed 4. Displays Axios error message instead of basic HTTP response --------- Signed-off-by: “khandrew-redhat” <“khandrew@redhat.com”> Co-authored-by: “khandrew-redhat” <“khandrew@redhat.com”>
1 parent 06df295 commit 623cdf7

File tree

10 files changed

+519
-343
lines changed

10 files changed

+519
-343
lines changed

.gitignore

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
.idea
2+
**/.DS_Store
23
backend/ocpperf.toml

backend/scripts/version.py

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,9 +10,8 @@
1010
from pathlib import Path
1111
import subprocess
1212
import sys
13-
from typing import Optional
14-
1513
import tomllib
14+
from typing import Optional
1615

1716

1817
def do(cmd: list[str]) -> list[str]:

frontend/package-lock.json

Lines changed: 82 additions & 324 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

frontend/src/actions/headerActions.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,6 @@ import * as API_ROUTES from "@/utils/apiConstants";
22
import * as TYPES from "./types.js";
33

44
import API from "@/utils/axiosInstance";
5-
import { showFailureToast } from "./toastActions.js";
65

76
export const setLastUpdatedTime = () => ({
87
type: TYPES.SET_LAST_UPDATED_TIME,
@@ -20,6 +19,7 @@ export const fetchAggregatorVersion = () => async (dispatch) => {
2019
});
2120
}
2221
} catch (error) {
23-
dispatch(showFailureToast());
22+
// Error handling is done automatically by axios interceptor
23+
console.error('Failed to fetch aggregator version:', error);
2424
}
2525
};

frontend/src/actions/quayActions.js

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@ import {
1616
import API from "@/utils/axiosInstance";
1717
import { cloneDeep } from "lodash";
1818
import { setLastUpdatedTime } from "./headerActions";
19-
import { showFailureToast } from "@/actions/toastActions";
2019
import { OTHERS } from "@/assets/constants/jobStatusConstants";
2120

2221
export const fetchQuayJobsData = () => async (dispatch) => {
@@ -256,7 +255,8 @@ export const fetchGraphData = (uuid) => async (dispatch, getState) => {
256255
}
257256
}
258257
} catch (error) {
259-
dispatch(showFailureToast());
258+
// Error handling is done automatically by axios interceptor
259+
console.error('Failed to fetch aggregator version:', error);
260260
}
261261
dispatch({ type: TYPES.GRAPH_COMPLETED });
262262
};

frontend/src/components/organisms/ToastComponent/index.jsx

Lines changed: 20 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
import { useDispatch, useSelector } from "react-redux";
88

99
import { hideToast } from "@/actions/toastActions";
10+
import "./index.less";
1011

1112
const ToastComponent = () => {
1213
const { alerts } = useSelector((state) => state.toast);
@@ -15,15 +16,32 @@ const ToastComponent = () => {
1516
const removeToast = (key) => {
1617
dispatch(hideToast(key));
1718
};
19+
20+
// Different timeout durations based on alert type
21+
const getTimeoutDuration = (variant) => {
22+
switch (variant) {
23+
case 'success':
24+
return 2500; // Success messages can disappear quickly
25+
case 'info':
26+
return 3000; // Info messages - standard duration
27+
case 'warning':
28+
return 4000; // Warnings need a bit more time
29+
case 'danger':
30+
return 4500; // Error messages need more time to read
31+
default:
32+
return 3000; // Default fallback
33+
}
34+
};
1835
return (
19-
<AlertGroup isToast>
36+
<AlertGroup isToast className="fast-fade-toast-group">
2037
{alerts.map((item) => (
2138
<Alert
2239
variant={AlertVariant[item.variant]}
2340
title={item.title}
2441
key={item.key}
25-
timeout={true}
42+
timeout={getTimeoutDuration(item.variant)}
2643
onTimeout={() => removeToast(item.key)}
44+
className="fast-fade-toast"
2745
actionClose={
2846
<AlertActionCloseButton
2947
title={item.title}
Lines changed: 101 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,101 @@
1+
// Assisted-by: Cursor AI
2+
// Toast animation styles for faster, smoother notifications
3+
4+
.fast-fade-toast-group {
5+
// Ensure proper z-index for toasts
6+
z-index: 9999;
7+
8+
// Animation for toast group
9+
.pf-c-alert-group__item {
10+
animation: slideInRight 0.3s ease-out;
11+
transition: all 0.3s ease-out;
12+
}
13+
}
14+
15+
.fast-fade-toast {
16+
// Faster transitions for all toast interactions
17+
transition: all 0.3s ease-out !important;
18+
19+
// Smoother entrance animation
20+
animation: slideInRight 0.3s ease-out;
21+
22+
// Override PatternFly's default fade-out animation to be faster
23+
&.pf-m-fadeout {
24+
animation: fadeOutRight 0.3s ease-in forwards !important;
25+
opacity: 0;
26+
transform: translateX(100%);
27+
}
28+
29+
// Hover effects for better UX
30+
&:hover {
31+
box-shadow: 0 4px 8px rgba(0, 0, 0, 0.15);
32+
transform: translateY(-1px);
33+
}
34+
}
35+
36+
// Keyframes for smooth animations
37+
@keyframes slideInRight {
38+
from {
39+
opacity: 0;
40+
transform: translateX(100%);
41+
}
42+
to {
43+
opacity: 1;
44+
transform: translateX(0);
45+
}
46+
}
47+
48+
@keyframes fadeOutRight {
49+
from {
50+
opacity: 1;
51+
transform: translateX(0);
52+
}
53+
to {
54+
opacity: 0;
55+
transform: translateX(100%);
56+
}
57+
}
58+
59+
// Additional improvements for toast appearance
60+
.fast-fade-toast {
61+
// Slightly more compact padding for quicker reading
62+
.pf-c-alert__title {
63+
font-weight: 600;
64+
margin-bottom: 4px;
65+
}
66+
67+
// Better spacing for message content
68+
p {
69+
margin: 2px 0;
70+
line-height: 1.4;
71+
}
72+
73+
// Smoother close button interaction
74+
.pf-c-button.pf-m-plain {
75+
transition: all 0.2s ease;
76+
77+
&:hover {
78+
background-color: rgba(255, 255, 255, 0.1);
79+
transform: scale(1.1);
80+
}
81+
}
82+
}
83+
84+
// Responsive behavior for mobile
85+
@media (max-width: 768px) {
86+
.fast-fade-toast-group {
87+
// Ensure toasts are readable on mobile
88+
.pf-c-alert-group__item {
89+
margin: 0 8px;
90+
}
91+
}
92+
93+
.fast-fade-toast {
94+
// Slightly smaller on mobile
95+
font-size: 0.9rem;
96+
97+
.pf-c-alert__title {
98+
font-size: 1rem;
99+
}
100+
}
101+
}
Lines changed: 161 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,161 @@
1+
import { describe, it, expect, test } from "vitest";
2+
3+
import {
4+
extractErrorMessage,
5+
} from "../errorUtils";
6+
7+
// Test the error priority logic that would be used in axios interceptor
8+
const mockErrorPrioritization = (axiosMessage, responseData, status = 500) => {
9+
const responseMessage = extractErrorMessage(responseData, status);
10+
11+
// Use response message if available, otherwise fall back to axios message
12+
const extractedMessage = responseMessage || axiosMessage;
13+
14+
if (extractedMessage) {
15+
return {
16+
message: extractedMessage,
17+
source: responseMessage ? 'Response data extraction' : 'Axios error message'
18+
};
19+
}
20+
21+
return { message: null, source: 'none' };
22+
};
23+
24+
describe("extractErrorMessage", () => {
25+
// Status code based enhanced messages (any text gets enhanced for these status codes)
26+
test.each([
27+
["Internal Server Error", 500, "A backend service is temporarily unavailable."],
28+
["Erreur interne du serveur", 500, "A backend service is temporarily unavailable."], // French
29+
["Custom 500 error", 500, "A backend service is temporarily unavailable."],
30+
["Bad Gateway", 502, "Unable to connect to backend service. Please try again in a moment."],
31+
["Proxy Error", 502, "Unable to connect to backend service. Please try again in a moment."],
32+
["Service Unavailable", 503, "The service is temporarily overloaded or under maintenance. Please try again later."],
33+
["Temporarily Unavailable", 503, "The service is temporarily overloaded or under maintenance. Please try again later."],
34+
["Gateway Timeout", 504, "The request timed out while connecting to external services. Please try again."],
35+
["Request Timeout", 504, "The request timed out while connecting to external services. Please try again."]
36+
])('enhances any plain text for status %d: "%s" -> enhanced message', (input, status, expected) => {
37+
expect(extractErrorMessage(input, status)).toBe(expected);
38+
});
39+
40+
// Non-enhanced status codes return original text
41+
test.each([
42+
["Custom server error", 400, "Custom server error"],
43+
["Not Found", 404, "Not Found"],
44+
["Unauthorized", 401, "Unauthorized"],
45+
[" Whitespace trimmed ", 422, "Whitespace trimmed"]
46+
])('returns original text for non-enhanced status %d: "%s"', (input, status, expected) => {
47+
expect(extractErrorMessage(input, status)).toBe(expected);
48+
});
49+
50+
// FastAPI error formats
51+
test.each([
52+
[{ detail: { message: "Custom error message" } }, 400, "Custom error message"],
53+
[{ detail: { error: "Error object format" } }, 422, "Error object format"],
54+
[{ detail: "String detail format" }, 400, "String detail format"],
55+
[{ error: "Direct error key format" }, 400, "Direct error key format"],
56+
[{ message: "Generic message field" }, 400, "Generic message field"]
57+
])('extracts from various JSON formats', (input, status, expected) => {
58+
expect(extractErrorMessage(input, status)).toBe(expected);
59+
});
60+
61+
// Null/invalid inputs
62+
test.each([
63+
[null], [undefined], [{}], [123], [[]], [""], [" "], [{ detail: [] }]
64+
])('returns enhanced message for status 500 even with unparseable data: %s', (input) => {
65+
expect(extractErrorMessage(input, 500)).toBe("A backend service is temporarily unavailable.");
66+
});
67+
68+
it("handles validation errors with field paths", () => {
69+
const result = extractErrorMessage({
70+
detail: [
71+
{ msg: "Field required", loc: ["query", "start_date"] },
72+
{ msg: "ensure this value is greater than 0", loc: ["query", "size"] },
73+
],
74+
}, 422);
75+
expect(result).toContain("Multiple validation errors");
76+
expect(result).toContain("Field required");
77+
expect(result).toContain("greater than 0");
78+
});
79+
80+
it("handles single validation error with nested path", () => {
81+
const result = extractErrorMessage({
82+
detail: [{ msg: "Field is required", loc: ["query", "start_date", "nested"] }],
83+
}, 422);
84+
expect(result).toBe("Field is required (start_date.nested)");
85+
});
86+
});
87+
88+
89+
describe("Error Prioritization Logic", () => {
90+
test.each([
91+
["Request failed with status code 422", { detail: { message: "Database connection timeout" } }, "Database connection timeout", "Response data extraction"],
92+
["Request failed with status code 500", "Internal Server Error", "A backend service is temporarily unavailable.", "Response data extraction"],
93+
["Network Error", "Internal Server Error", "A backend service is temporarily unavailable.", "Response data extraction"],
94+
["Connection refused to database server", "Internal Server Error", "A backend service is temporarily unavailable.", "Response data extraction"],
95+
["timeout of 5000ms exceeded", "Internal Server Error", "A backend service is temporarily unavailable.", "Response data extraction"],
96+
[null, { detail: { message: "Specific error" } }, "Specific error", "Response data extraction"],
97+
[undefined, "Internal Server Error", "A backend service is temporarily unavailable.", "Response data extraction"],
98+
["Network Error", { random: "data" }, "A backend service is temporarily unavailable.", "Response data extraction"],
99+
["", "Internal Server Error", "A backend service is temporarily unavailable.", "Response data extraction"]
100+
])('prioritization: axios="%s" response=%j -> message="%s" source="%s"', (axiosMessage, responseData, expectedMessage, expectedSource) => {
101+
const result = mockErrorPrioritization(axiosMessage, responseData, responseData && typeof responseData === 'object' && responseData.detail ? 422 : 500);
102+
expect(result.message).toBe(expectedMessage);
103+
expect(result.source).toBe(expectedSource);
104+
});
105+
106+
it("prefers validation errors over axios messages", () => {
107+
const result = mockErrorPrioritization(
108+
"Request failed with status code 422",
109+
{ detail: [{ msg: "Field required", loc: ["query", "start_date"] }] },
110+
422
111+
);
112+
113+
expect(result).toEqual({
114+
message: "Field required (start_date)",
115+
source: "Response data extraction"
116+
});
117+
});
118+
});
119+
120+
describe("Integration Tests - Full Error Handling Flow", () => {
121+
const mockFullErrorHandling = (status, data, url, axiosMessage) => {
122+
// Note: Current implementation shows toasts for ALL status codes (400-599)
123+
const responseMessage = extractErrorMessage(data, status);
124+
125+
// Use response message if available, otherwise fall back to axios message
126+
const extractedMessage = responseMessage || axiosMessage;
127+
const messageSource = responseMessage ? 'Response data extraction' : 'Axios error message';
128+
129+
return {
130+
showToast: true, // Now shows for all HTTP errors
131+
message: extractedMessage,
132+
source: messageSource
133+
};
134+
};
135+
136+
test.each([
137+
[500, "Internal Server Error", "/api/v1/telco/filters", "Request failed with status code 500", "A backend service is temporarily unavailable.", "Response data extraction"],
138+
[422, { detail: [{ msg: "Field required", loc: ["query", "start_date"] }] }, "/api/v1/ocp/jobs", "Request failed with status code 422", "Field required (start_date)", "Response data extraction"],
139+
[400, { detail: { message: "Invalid date range" } }, "/api/v1/quay/jobs", "Request failed with status code 400", "Invalid date range", "Response data extraction"],
140+
[404, { detail: "Not found" }, "/api/v1/ocp/jobs", "Request failed with status code 404", "Not found", "Response data extraction"]
141+
])('status %d with %j -> message="%s" source="%s"', (status, data, url, axiosMessage, expectedMessage, expectedSource) => {
142+
const result = mockFullErrorHandling(status, data, url, axiosMessage);
143+
144+
expect(result).toEqual({
145+
showToast: true,
146+
message: expectedMessage,
147+
source: expectedSource
148+
});
149+
});
150+
151+
it("handles enhanced error messages", () => {
152+
const result = mockFullErrorHandling(
153+
504,
154+
"Gateway Timeout",
155+
"/api/v1/ols/jobs",
156+
"timeout of 30000ms exceeded"
157+
);
158+
159+
expect(result.message).toBe("The request timed out while connecting to external services. Please try again.");
160+
});
161+
});

0 commit comments

Comments
 (0)