Skip to content

Commit 5c52173

Browse files
authored
bug: Fix mix of camel case and title case in error messages (#2314)
* Port error message camel to title case formatter to frontend * Add tests * Update error message formatter to use a map * f
1 parent 9825d66 commit 5c52173

File tree

4 files changed

+119
-77
lines changed

4 files changed

+119
-77
lines changed

api/integration/install_test.go

Lines changed: 3 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -418,7 +418,7 @@ func TestConfigureInstallationValidation(t *testing.T) {
418418
var apiError types.APIError
419419
err = json.NewDecoder(rec.Body).Decode(&apiError)
420420
require.NoError(t, err)
421-
assert.Contains(t, apiError.Error(), "Service CIDR is required when globalCidr is not set")
421+
assert.Contains(t, apiError.Error(), "serviceCidr is required when globalCidr is not set")
422422
// Also verify the field name is correct
423423
assert.Equal(t, "serviceCidr", apiError.Errors[0].Field)
424424
}
@@ -1085,12 +1085,8 @@ func TestInstallWithAPIClient(t *testing.T) {
10851085
apiErr, ok := err.(*types.APIError)
10861086
require.True(t, ok, "Error should be of type *types.APIError")
10871087
assert.Equal(t, http.StatusBadRequest, apiErr.StatusCode)
1088-
// Error message should contain both variants of the port conflict message
1089-
assert.True(t,
1090-
strings.Contains(apiErr.Error(), "Admin Console Port and localArtifactMirrorPort cannot be equal") &&
1091-
strings.Contains(apiErr.Error(), "adminConsolePort and Local Artifact Mirror Port cannot be equal"),
1092-
"Error message should contain both variants of the port conflict message",
1093-
)
1088+
// Error message should contain the same port conflict message for both fields
1089+
assert.Equal(t, 2, strings.Count(apiErr.Error(), "adminConsolePort and localArtifactMirrorPort cannot be equal"))
10941090
})
10951091

10961092
// Test SetInstallStatus

api/types/errors.go

Lines changed: 3 additions & 61 deletions
Original file line numberDiff line numberDiff line change
@@ -6,11 +6,6 @@ import (
66
"errors"
77
"fmt"
88
"net/http"
9-
"regexp"
10-
"strings"
11-
12-
"golang.org/x/text/cases"
13-
"golang.org/x/text/language"
149
)
1510

1611
type APIError struct {
@@ -105,64 +100,11 @@ func AppendFieldError(apiErr *APIError, field string, err error) *APIError {
105100
if apiErr == nil {
106101
apiErr = NewBadRequestError(errors.New("field errors"))
107102
}
108-
return AppendError(apiErr, newFieldError(field, err))
109-
}
110-
111-
func camelCaseToWords(s string) string {
112-
// Handle special cases
113-
specialCases := map[string]string{
114-
"cidr": "CIDR",
115-
"Cidr": "CIDR",
116-
"CIDR": "CIDR",
117-
}
118-
119-
// Check if the entire string is a special case
120-
if replacement, ok := specialCases[strings.ToLower(s)]; ok {
121-
return replacement
122-
}
123-
124-
// Split on capital letters
125-
re := regexp.MustCompile(`([a-z])([A-Z])`)
126-
words := re.ReplaceAllString(s, "$1 $2")
127-
128-
// Split the words and handle special cases
129-
wordList := strings.Split(strings.ToLower(words), " ")
130-
for i, word := range wordList {
131-
if replacement, ok := specialCases[word]; ok {
132-
wordList[i] = replacement
133-
} else {
134-
// Capitalize other words
135-
c := cases.Title(language.English)
136-
wordList[i] = c.String(word)
137-
}
138-
}
139-
140-
return strings.Join(wordList, " ")
141-
}
142-
143-
func newFieldError(field string, err error) *APIError {
144-
msg := err.Error()
145-
146-
// Try different patterns to replace the field name
147-
patterns := []string{
148-
field, // exact match
149-
strings.ToLower(field), // lowercase
150-
strings.ToUpper(field), // uppercase
151-
"cidr", // special case for CIDR
152-
}
153-
154-
for _, pattern := range patterns {
155-
if strings.Contains(msg, pattern) {
156-
msg = strings.Replace(msg, pattern, camelCaseToWords(field), 1)
157-
break
158-
}
159-
}
160-
161-
return &APIError{
162-
Message: msg,
103+
return AppendError(apiErr, &APIError{
104+
Message: err.Error(),
163105
Field: field,
164106
err: err,
165-
}
107+
})
166108
}
167109

168110
// JSON writes the APIError as JSON to the provided http.ResponseWriter

web/src/components/wizard/setup/LinuxSetup.tsx

Lines changed: 47 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,27 @@ import Select from "../../common/Select";
44
import { useBranding } from "../../../contexts/BrandingContext";
55
import { ChevronDown, ChevronRight } from "lucide-react";
66

7+
/**
8+
* Maps internal field names to user-friendly display names.
9+
* Used for:
10+
* - Input IDs: <Input id="adminConsolePort" />
11+
* - Labels: <Input label={fieldNames.adminConsolePort} />
12+
* - Error formatting: formatErrorMessage("adminConsolePort invalid") -> "Admin Console Port invalid"
13+
*/
14+
const fieldNames = {
15+
adminConsolePort: "Admin Console Port",
16+
dataDirectory: "Data Directory",
17+
localArtifactMirrorPort: "Local Artifact Mirror Port",
18+
httpProxy: "HTTP Proxy",
19+
httpsProxy: "HTTPS Proxy",
20+
noProxy: "Proxy Bypass List",
21+
networkInterface: "Network Interface",
22+
podCidr: "Pod CIDR",
23+
serviceCidr: "Service CIDR",
24+
globalCidr: "Reserved Network Range (CIDR)",
25+
cidr: "CIDR",
26+
}
27+
728
interface LinuxSetupProps {
829
config: {
930
dataDirectory?: string;
@@ -43,7 +64,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
4364

4465
const getFieldError = (fieldName: string) => {
4566
const fieldError = fieldErrors.find((err) => err.field === fieldName);
46-
return fieldError?.message;
67+
return fieldError ? formatErrorMessage(fieldError.message) : undefined;
4768
};
4869

4970
return (
@@ -52,7 +73,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
5273
<h2 className="text-lg font-medium text-gray-900">System Configuration</h2>
5374
<Input
5475
id="dataDirectory"
55-
label="Data Directory"
76+
label={fieldNames.dataDirectory}
5677
value={config.dataDirectory || ""}
5778
onChange={onInputChange}
5879
placeholder="/var/lib/embedded-cluster"
@@ -63,7 +84,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
6384

6485
<Input
6586
id="adminConsolePort"
66-
label="Admin Console Port"
87+
label={fieldNames.adminConsolePort}
6788
value={config.adminConsolePort?.toString() || ""}
6889
onChange={onInputChange}
6990
placeholder="30000"
@@ -74,7 +95,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
7495

7596
<Input
7697
id="localArtifactMirrorPort"
77-
label="Local Artifact Mirror Port"
98+
label={fieldNames.localArtifactMirrorPort}
7899
value={config.localArtifactMirrorPort?.toString() || ""}
79100
onChange={onInputChange}
80101
placeholder="50000"
@@ -89,7 +110,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
89110
<div className="space-y-4">
90111
<Input
91112
id="httpProxy"
92-
label="HTTP Proxy"
113+
label={fieldNames.httpProxy}
93114
value={config.httpProxy || ""}
94115
onChange={onInputChange}
95116
placeholder="http://proxy.example.com:3128"
@@ -99,7 +120,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
99120

100121
<Input
101122
id="httpsProxy"
102-
label="HTTPS Proxy"
123+
label={fieldNames.httpsProxy}
103124
value={config.httpsProxy || ""}
104125
onChange={onInputChange}
105126
placeholder="https://proxy.example.com:3128"
@@ -109,7 +130,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
109130

110131
<Input
111132
id="noProxy"
112-
label="Proxy Bypass List"
133+
label={fieldNames.noProxy}
113134
value={config.noProxy || ""}
114135
onChange={onInputChange}
115136
placeholder="localhost,127.0.0.1,.example.com"
@@ -133,7 +154,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
133154
<div className="space-y-6">
134155
<Select
135156
id="networkInterface"
136-
label="Network Interface"
157+
label={fieldNames.networkInterface}
137158
value={config.networkInterface || ""}
138159
onChange={onSelectChange}
139160
options={[
@@ -155,7 +176,7 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
155176

156177
<Input
157178
id="globalCidr"
158-
label="Reserved Network Range (CIDR)"
179+
label={fieldNames.globalCidr}
159180
value={config.globalCidr || ""}
160181
onChange={onInputChange}
161182
placeholder="10.244.0.0/16"
@@ -170,4 +191,21 @@ const LinuxSetup: React.FC<LinuxSetupProps> = ({
170191
);
171192
};
172193

194+
/**
195+
* Formats error messages by replacing technical field names with more user-friendly display names.
196+
* Example: "adminConsolePort" becomes "Admin Console Port".
197+
*
198+
* @param message - The error message to format
199+
* @returns The formatted error message with replaced field names
200+
*/
201+
export function formatErrorMessage(message: string) {
202+
let finalMsg = message
203+
for (const [field, fieldName] of Object.entries(fieldNames)) {
204+
// Case-insensitive regex that matches whole words only
205+
// Example: "podCidr", "PodCidr", "PODCIDR" all become "Pod CIDR"
206+
finalMsg = finalMsg.replace(new RegExp(`\\b${field}\\b`, 'gi'), fieldName)
207+
}
208+
return finalMsg
209+
}
210+
173211
export default LinuxSetup;
Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,66 @@
1+
import { describe, it, expect } from "vitest";
2+
import { formatErrorMessage } from "../setup/LinuxSetup";
3+
4+
describe("formatErrorMessage", () => {
5+
it("handles empty string", () => {
6+
expect(formatErrorMessage("")).toBe("");
7+
});
8+
9+
it("replaces field names with their proper format", () => {
10+
expect(formatErrorMessage("adminConsolePort")).toBe("Admin Console Port");
11+
expect(formatErrorMessage("dataDirectory")).toBe("Data Directory");
12+
expect(formatErrorMessage("localArtifactMirrorPort")).toBe("Local Artifact Mirror Port");
13+
expect(formatErrorMessage("httpProxy")).toBe("HTTP Proxy");
14+
expect(formatErrorMessage("httpsProxy")).toBe("HTTPS Proxy");
15+
expect(formatErrorMessage("noProxy")).toBe("Proxy Bypass List");
16+
expect(formatErrorMessage("networkInterface")).toBe("Network Interface");
17+
expect(formatErrorMessage("podCidr")).toBe("Pod CIDR");
18+
expect(formatErrorMessage("serviceCidr")).toBe("Service CIDR");
19+
expect(formatErrorMessage("globalCidr")).toBe("Reserved Network Range (CIDR)");
20+
expect(formatErrorMessage("cidr")).toBe("CIDR");
21+
});
22+
23+
it("handles multiple field names in one message", () => {
24+
expect(formatErrorMessage("podCidr and serviceCidr are required")).toBe("Pod CIDR and Service CIDR are required");
25+
expect(formatErrorMessage("httpProxy and httpsProxy must be set")).toBe("HTTP Proxy and HTTPS Proxy must be set");
26+
});
27+
28+
it("preserves non-field words", () => {
29+
expect(formatErrorMessage("The podCidr is invalid")).toBe("The Pod CIDR is invalid");
30+
expect(formatErrorMessage("Please set the httpProxy")).toBe("Please set the HTTP Proxy");
31+
});
32+
33+
it("handles case insensitivity correctly", () => {
34+
expect(formatErrorMessage("PodCidr")).toBe("Pod CIDR");
35+
expect(formatErrorMessage("HTTPPROXY")).toBe("HTTP Proxy");
36+
expect(formatErrorMessage("cidr")).toBe("CIDR");
37+
expect(formatErrorMessage("Cidr")).toBe("CIDR");
38+
expect(formatErrorMessage("CIDR")).toBe("CIDR");
39+
});
40+
41+
it("handles real-world error messages", () => {
42+
expect(formatErrorMessage("The podCidr 10.0.0.0/24 overlaps with serviceCidr 10.0.0.0/16")).toBe(
43+
"The Pod CIDR 10.0.0.0/24 overlaps with Service CIDR 10.0.0.0/16"
44+
);
45+
expect(formatErrorMessage("httpProxy and httpsProxy cannot be empty when noProxy is set")).toBe(
46+
"HTTP Proxy and HTTPS Proxy cannot be empty when Proxy Bypass List is set"
47+
);
48+
expect(formatErrorMessage("adminConsolePort must be between 1024 and 65535")).toBe(
49+
"Admin Console Port must be between 1024 and 65535"
50+
);
51+
expect(formatErrorMessage("dataDirectory /var/lib/k0s is not writable")).toBe(
52+
"Data Directory /var/lib/k0s is not writable"
53+
);
54+
expect(formatErrorMessage("globalCidr must be a valid CIDR block")).toBe(
55+
"Reserved Network Range (CIDR) must be a valid CIDR block"
56+
);
57+
});
58+
59+
it("handles special characters and formatting", () => {
60+
expect(formatErrorMessage("admin_console_port and localArtifactMirrorPort cannot be equal.")).toBe(
61+
"admin_console_port and Local Artifact Mirror Port cannot be equal."
62+
);
63+
expect(formatErrorMessage("httpProxy: invalid URL format")).toBe("HTTP Proxy: invalid URL format");
64+
expect(formatErrorMessage("podCidr: 192.168.0.0/24 (invalid)")).toBe("Pod CIDR: 192.168.0.0/24 (invalid)");
65+
});
66+
});

0 commit comments

Comments
 (0)