Skip to content

Commit dbb22ff

Browse files
committed
fix(FR-2355): handle duplicate TOML keys in config.toml parsing
The Backend.AI webserver generates config.toml with duplicate keys (e.g. two `debug = true` lines), which causes smol-toml to throw. The error was silently swallowed, falling back to defaults and making all config options appear disabled. - Add deduplicateTomlKeys() preprocessing to remove duplicate keys before TOML parsing (keeps last occurrence) - Change fetchAndParseConfig return type to { config, error } to distinguish fetch failure from parse failure - Add useBAILogger-based error logging for parse failures - Update all call sites for new return type - Add test for duplicate key handling
1 parent c066bf6 commit dbb22ff

27 files changed

+203
-63
lines changed

react/src/components/DiagnosticResultList.tsx

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44
*/
55
import type { DiagnosticResult } from '../types/diagnostics';
66
import { Alert, Skeleton, theme, Typography } from 'antd';
7+
import Paragraph from 'antd/lib/typography/Paragraph';
78
import { BAIFlex } from 'backend.ai-ui';
89
import { CheckCircle } from 'lucide-react';
910
import { useTranslation } from 'react-i18next';
@@ -54,6 +55,11 @@ const DiagnosticResultList: React.FC<DiagnosticResultListProps> = ({
5455
<span>
5556
{t(result.descriptionKey, result.interpolationValues)}
5657
</span>
58+
{result.interpolationValues?.errorMessage && (
59+
<Paragraph>
60+
<pre>{result.interpolationValues.errorMessage}</pre>
61+
</Paragraph>
62+
)}
5763
{result.remediationKey && (
5864
<span style={{ fontStyle: 'italic' }}>
5965
{t(result.remediationKey, result.interpolationValues)}

react/src/components/EduAppLauncher.tsx

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ const EduAppLauncher: React.FC<EduAppLauncherProps> = ({
8888
'Backend.AI Web UI.',
8989
);
9090
const configPath = g.isElectron ? './config.toml' : '../../config.toml';
91-
const tomlConfig = await fetchAndParseConfig(configPath);
91+
const { config: tomlConfig } = await fetchAndParseConfig(configPath);
9292
if (tomlConfig?.wsproxy?.proxyURL) {
9393
g.backendaiclient._config._proxyURL = tomlConfig.wsproxy.proxyURL;
9494
}

react/src/helper/loginSessionAuth.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -204,7 +204,7 @@ export async function loadConfigFromWebServer(
204204
return;
205205
}
206206
const webserverConfigURL = new URL('./config.toml', apiEndpoint).href;
207-
const config = await fetchAndParseConfig(webserverConfigURL);
207+
const { config } = await fetchAndParseConfig(webserverConfigURL);
208208
if (!config) return;
209209

210210
const backendaiutils = (globalThis as Record<string, any>).backendaiutils;

react/src/hooks/useWebServerConfigDiagnostics.ts

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,11 @@ import {
1616
isPlaceholder,
1717
} from '../diagnostics/rules/configRules';
1818
import type { DiagnosticResult } from '../types/diagnostics';
19-
import { useProxyUrl, useRawConfig } from './useWebUIConfig';
19+
import {
20+
useConfigParseError,
21+
useProxyUrl,
22+
useRawConfig,
23+
} from './useWebUIConfig';
2024
import { VALID_MENU_KEYS } from './useWebUIMenuItems';
2125

2226
/**
@@ -32,8 +36,28 @@ export function useWebServerConfigDiagnostics(
3236
const baiClient = useSuspendedBackendaiClient();
3337
const rawConfig = useRawConfig();
3438
const proxyUrl = useProxyUrl();
39+
const configParseError = useConfigParseError();
3540

3641
const results: DiagnosticResult[] = [];
42+
43+
// --- config.toml parse failure (e.g. duplicate keys) ---
44+
if (configParseError) {
45+
results.push({
46+
id: 'config-parse-error',
47+
severity: 'critical',
48+
category: 'config',
49+
titleKey: 'diagnostics.ConfigParseError',
50+
descriptionKey: 'diagnostics.ConfigParseErrorDesc',
51+
interpolationValues: {
52+
errorMessage:
53+
configParseError instanceof Error
54+
? configParseError.message
55+
: String(configParseError),
56+
},
57+
});
58+
return results;
59+
}
60+
3761
const apiEndpoint: string = baiClient?._config?.endpoint ?? '';
3862
const blockList: string[] = baiClient?._config?.blockList ?? [];
3963

react/src/hooks/useWebUIConfig.test.ts

Lines changed: 58 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -49,32 +49,42 @@ describe('fetchAndParseConfig', () => {
4949
jest.restoreAllMocks();
5050
});
5151

52-
it('returns null when fetch response status is not 200', async () => {
52+
it('returns null config when fetch response status is not 200', async () => {
5353
mockFetch(404, '');
5454
const result = await fetchAndParseConfig('/config.toml');
55-
expect(result).toBeNull();
55+
expect(result.config).toBeNull();
56+
expect(result.error).toBeUndefined();
5657
});
5758

58-
it('returns null when fetch throws an error', async () => {
59+
it('returns null config without error when fetch throws (network failure)', async () => {
5960
global.fetch = jest.fn().mockRejectedValue(new Error('Network error'));
6061
const result = await fetchAndParseConfig('/config.toml');
61-
expect(result).toBeNull();
62+
expect(result.config).toBeNull();
63+
expect(result.error).toBeUndefined();
6264
});
6365

64-
it('returns null for HTTP 500 (server error)', async () => {
66+
it('returns null config with error when TOML parsing fails', async () => {
67+
// Invalid TOML that will cause a parse error
68+
mockFetch(200, '[general]\ninvalid toml = = =');
69+
const result = await fetchAndParseConfig('/config.toml');
70+
expect(result.config).toBeNull();
71+
expect(result.error).toBeDefined();
72+
});
73+
74+
it('returns null config for HTTP 500 (server error)', async () => {
6575
mockFetch(500, 'Internal Server Error');
6676
const result = await fetchAndParseConfig('/config.toml');
67-
expect(result).toBeNull();
77+
expect(result.config).toBeNull();
6878
});
6979

70-
it('returns null when response Content-Type is text/html (SPA fallback)', async () => {
80+
it('returns null config when response Content-Type is text/html (SPA fallback)', async () => {
7181
// When config.toml is missing, the dev server's historyApiFallback
7282
// serves index.html with status 200 and Content-Type: text/html.
7383
const html =
7484
'<!DOCTYPE html><html><head><title>App</title></head><body></body></html>';
7585
mockFetch(200, html, 'text/html; charset=utf-8');
7686
const result = await fetchAndParseConfig('/config.toml');
77-
expect(result).toBeNull();
87+
expect(result.config).toBeNull();
7888
});
7989

8090
it('returns parsed config object for valid TOML', async () => {
@@ -84,9 +94,9 @@ apiEndpoint = "https://api.example.com"
8494
`;
8595
mockFetch(200, tomlContent);
8696
const result = await fetchAndParseConfig('/config.toml');
87-
expect(result).not.toBeNull();
88-
expect(result?.general).toBeDefined();
89-
expect(result?.general?.apiEndpoint).toBe('https://api.example.com');
97+
expect(result.config).not.toBeNull();
98+
expect(result.config?.general).toBeDefined();
99+
expect(result.config?.general?.apiEndpoint).toBe('https://api.example.com');
90100
});
91101

92102
it('parses license section from TOML', async () => {
@@ -97,9 +107,9 @@ validUntil = "2030-12-31"
97107
`;
98108
mockFetch(200, tomlContent);
99109
const result = await fetchAndParseConfig('/config.toml');
100-
expect(result).not.toBeNull();
101-
expect(result?.license?.edition).toBe('Enterprise');
102-
expect(result?.license?.validUntil).toBe('2030-12-31');
110+
expect(result.config).not.toBeNull();
111+
expect(result.config?.license?.edition).toBe('Enterprise');
112+
expect(result.config?.license?.validUntil).toBe('2030-12-31');
103113
});
104114

105115
it('parses wsproxy section from TOML', async () => {
@@ -109,8 +119,8 @@ proxyURL = "http://127.0.0.1:5050/"
109119
`;
110120
mockFetch(200, tomlContent);
111121
const result = await fetchAndParseConfig('/config.toml');
112-
expect(result).not.toBeNull();
113-
expect(result?.wsproxy?.proxyURL).toBe('http://127.0.0.1:5050/');
122+
expect(result.config).not.toBeNull();
123+
expect(result.config?.wsproxy?.proxyURL).toBe('http://127.0.0.1:5050/');
114124
});
115125

116126
it('parses plugin section from TOML', async () => {
@@ -121,9 +131,9 @@ page = "custom-pages"
121131
`;
122132
mockFetch(200, tomlContent);
123133
const result = await fetchAndParseConfig('/config.toml');
124-
expect(result).not.toBeNull();
125-
expect(result?.plugin?.login).toBe('custom-login');
126-
expect(result?.plugin?.page).toBe('custom-pages');
134+
expect(result.config).not.toBeNull();
135+
expect(result.config?.plugin?.login).toBe('custom-login');
136+
expect(result.config?.plugin?.page).toBe('custom-pages');
127137
});
128138

129139
it('preprocesses apiEndpointText escape sequences', async () => {
@@ -135,9 +145,9 @@ apiEndpointText = "Line1\\nLine2"
135145
`;
136146
mockFetch(200, tomlContent);
137147
const result = await fetchAndParseConfig('/config.toml');
138-
expect(result).not.toBeNull();
148+
expect(result.config).not.toBeNull();
139149
// After preprocessing, the string should contain an actual newline
140-
expect(result?.general?.apiEndpointText).toBe('Line1\nLine2');
150+
expect(result.config?.general?.apiEndpointText).toBe('Line1\nLine2');
141151
});
142152

143153
it('keeps original apiEndpointText when preprocessing fails', async () => {
@@ -148,9 +158,9 @@ apiEndpointText = "valid text"
148158
`;
149159
mockFetch(200, tomlContent);
150160
const result = await fetchAndParseConfig('/config.toml');
151-
expect(result).not.toBeNull();
161+
expect(result.config).not.toBeNull();
152162
// Regular text without escape sequences should be preserved
153-
expect(result?.general?.apiEndpointText).toBe('valid text');
163+
expect(result.config?.general?.apiEndpointText).toBe('valid text');
154164
});
155165

156166
it('handles TOML with multiple sections', async () => {
@@ -170,19 +180,32 @@ page = ""
170180
`;
171181
mockFetch(200, tomlContent);
172182
const result = await fetchAndParseConfig('/config.toml');
173-
expect(result).not.toBeNull();
174-
expect(result?.general?.apiEndpoint).toBe('https://api.example.com');
175-
expect(result?.license?.edition).toBe('Community');
176-
expect(result?.wsproxy?.proxyURL).toBe('http://localhost:5050');
183+
expect(result.config).not.toBeNull();
184+
expect(result.config?.general?.apiEndpoint).toBe('https://api.example.com');
185+
expect(result.config?.license?.edition).toBe('Community');
186+
expect(result.config?.wsproxy?.proxyURL).toBe('http://localhost:5050');
177187
});
178188

179-
it('returns null for empty TOML body', async () => {
189+
it('returns non-null config for empty TOML body', async () => {
180190
mockFetch(200, '');
181191
const result = await fetchAndParseConfig('/config.toml');
182-
// markty-toml parses empty string to an empty object, not null
192+
// smol-toml parses empty string to an empty object, not null
183193
// The config should be a non-null object (empty)
184-
expect(result).not.toBeNull();
185-
expect(typeof result).toBe('object');
194+
expect(result.config).not.toBeNull();
195+
expect(typeof result.config).toBe('object');
196+
});
197+
198+
it('returns error when TOML has duplicate keys', async () => {
199+
const tomlContent = `
200+
[general]
201+
debug = false
202+
apiEndpoint = "https://api.example.com"
203+
debug = true
204+
`;
205+
mockFetch(200, tomlContent);
206+
const result = await fetchAndParseConfig('/config.toml');
207+
expect(result.config).toBeNull();
208+
expect(result.error).toBeDefined();
186209
});
187210

188211
it('calls fetch with the provided config path', async () => {
@@ -216,7 +239,7 @@ edition = "Open Source"
216239
`;
217240
mockFetch(200, tomlContent);
218241
const result = await fetchAndParseConfig('/config.toml');
219-
expect(result?.general).toBeUndefined();
242+
expect(result.config?.general).toBeUndefined();
220243
});
221244

222245
it('does not modify config when apiEndpointText is absent', async () => {
@@ -226,8 +249,8 @@ apiEndpoint = "https://example.com"
226249
`;
227250
mockFetch(200, tomlContent);
228251
const result = await fetchAndParseConfig('/config.toml');
229-
expect(result?.general?.apiEndpoint).toBe('https://example.com');
230-
expect(result?.general?.apiEndpointText).toBeUndefined();
252+
expect(result.config?.general?.apiEndpoint).toBe('https://example.com');
253+
expect(result.config?.general?.apiEndpointText).toBeUndefined();
231254
});
232255

233256
it('handles tab escape sequence in apiEndpointText', async () => {
@@ -237,7 +260,7 @@ apiEndpointText = "Column1\\tColumn2"
237260
`;
238261
mockFetch(200, tomlContent);
239262
const result = await fetchAndParseConfig('/config.toml');
240-
expect(result?.general?.apiEndpointText).toBe('Column1\tColumn2');
263+
expect(result.config?.general?.apiEndpointText).toBe('Column1\tColumn2');
241264
});
242265
});
243266

0 commit comments

Comments
 (0)