Skip to content

Commit 4a6c2da

Browse files
open-design-crew[bot]Cheems
andauthored
fix(web): surface BYOK migration validation errors (#6280)
* fix(web): surface BYOK migration validation errors Preserve structured daemon errors during legacy credential migration and reserve offline guidance for fetch failures. Keep legacy browser credentials until migration succeeds, and cover HTTP validation, malformed responses, and network failures. Validated with focused web tests, web typecheck, workspace guard, and workspace typecheck. * fix(web): stabilize BYOK persistence error telemetry Map secure-profile HTTP and network failures to semantic error codes before emitting settings_byok_test_result, while preserving existing fallback behavior for unrelated connection-test errors. Validated with the complete web test suite, web typecheck, workspace typecheck, guard, and git diff --check. --------- Co-authored-by: Cheems <94773058+itscheems@users.noreply.github.com>
1 parent ebcba70 commit 4a6c2da

5 files changed

Lines changed: 311 additions & 13 deletions

File tree

apps/web/src/App.tsx

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -82,6 +82,7 @@ import {
8282
fetchMediaProvidersFromDaemon,
8383
hasAnyConfiguredProvider,
8484
fetchComposioConfigFromDaemon,
85+
legacyByokMigrationErrorPresentation,
8586
loadConfig,
8687
migrateLegacyByokCredentialsToDaemon,
8788
mergeDaemonConfig,
@@ -454,7 +455,7 @@ function AppInner() {
454455
const [workingDirError, setWorkingDirError] = useState<string | null>(null);
455456
const [projectOpenError, setProjectOpenError] = useState<string | null>(null);
456457
const [legacyByokMigrationError, setLegacyByokMigrationError] =
457-
useState<string | null>(null);
458+
useState<Error | null>(null);
458459
const [settingsWelcome, setSettingsWelcome] = useState(false);
459460
const [settingsInitialSection, setSettingsInitialSection] = useState<SettingsSection>('execution');
460461
const [settingsHighlight, setSettingsHighlight] = useState<SettingsHighlight>(null);
@@ -1013,7 +1014,7 @@ function AppInner() {
10131014
if (cancelled) return;
10141015
setLegacyByokMigrationError(
10151016
legacyByokMigration.status === 'failed'
1016-
? legacyByokMigration.error.message
1017+
? legacyByokMigration.error
10171018
: null,
10181019
);
10191020
const migrationBaseConfig = legacyByokMigration.config;
@@ -2629,6 +2630,12 @@ function AppInner() {
26292630
/>
26302631
);
26312632
}
2633+
const legacyByokMigrationErrorView = legacyByokMigrationError
2634+
? legacyByokMigrationErrorPresentation(
2635+
legacyByokMigrationError,
2636+
t('settings.autosaveError'),
2637+
)
2638+
: null;
26322639
return (
26332640
<>
26342641
<div
@@ -2739,10 +2746,10 @@ function AppInner() {
27392746
onDismiss={() => setProjectOpenError(null)}
27402747
/>
27412748
) : null}
2742-
{legacyByokMigrationError ? (
2749+
{legacyByokMigrationErrorView ? (
27432750
<Toast
2744-
message={t('settings.autosaveError')}
2745-
details={legacyByokMigrationError}
2751+
message={legacyByokMigrationErrorView.message}
2752+
details={legacyByokMigrationErrorView.details}
27462753
actionLabel={t('settings.title')}
27472754
onAction={() => {
27482755
setLegacyByokMigrationError(null);

apps/web/src/components/SettingsDialog.tsx

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -74,6 +74,7 @@ import {
7474
} from './modelOptions';
7575
import {
7676
BYOK_PROVIDER_PRESETS,
77+
classifyByokCredentialProfileFailure,
7778
DEFAULT_NOTIFICATIONS,
7879
DEFAULT_ORBIT,
7980
applySavedByokCredentialProfile,
@@ -2669,13 +2670,14 @@ export function SettingsDialog({
26692670
});
26702671
const byokProviderId = byokProtocolToTracking(apiProtocol);
26712672
if (byokProviderId) {
2673+
const failure = classifyByokCredentialProfileFailure(err);
26722674
trackSettingsByokTestResult(analytics.track, {
26732675
page_name: 'settings',
26742676
area: 'execution_model',
26752677
provider_id: byokProviderId,
26762678
result: 'failed',
2677-
error_code: err instanceof Error ? err.name : 'UNKNOWN',
2678-
error_kind: err instanceof Error ? err.name : 'UNKNOWN',
2679+
error_code: failure.errorCode,
2680+
error_kind: failure.errorKind,
26792681
field_missing: 'none',
26802682
config_key_changed: configKeyChanged,
26812683
success_after_action: false,

apps/web/src/state/config.ts

Lines changed: 101 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -848,16 +848,111 @@ export async function fetchByokCredentialProfilesFromDaemon(): Promise<
848848
}
849849
}
850850

851+
export class ByokCredentialProfileHttpError extends Error {
852+
readonly status: number;
853+
readonly code?: string;
854+
855+
constructor(status: number, message: string, code?: string) {
856+
super(message);
857+
this.name = 'ByokCredentialProfileHttpError';
858+
this.status = status;
859+
this.code = code;
860+
}
861+
}
862+
863+
export class ByokCredentialProfileNetworkError extends Error {
864+
constructor(message: string, cause: unknown) {
865+
super(message, { cause });
866+
this.name = 'ByokCredentialProfileNetworkError';
867+
}
868+
}
869+
870+
export function classifyByokCredentialProfileFailure(error: unknown): {
871+
errorCode: string;
872+
errorKind: string;
873+
} {
874+
if (error instanceof ByokCredentialProfileHttpError) {
875+
return {
876+
errorCode: error.code || `HTTP_${error.status}`,
877+
errorKind: 'unknown',
878+
};
879+
}
880+
if (error instanceof ByokCredentialProfileNetworkError) {
881+
return {
882+
errorCode: 'DAEMON_UNREACHABLE',
883+
errorKind: 'unknown',
884+
};
885+
}
886+
const fallback = error instanceof Error ? error.name : 'UNKNOWN';
887+
return { errorCode: fallback, errorKind: fallback };
888+
}
889+
890+
export function legacyByokMigrationErrorPresentation(
891+
error: Error,
892+
daemonUnavailableMessage: string,
893+
): { message: string; details?: string } {
894+
if (error instanceof ByokCredentialProfileNetworkError) {
895+
return {
896+
message: daemonUnavailableMessage,
897+
details: error.message,
898+
};
899+
}
900+
return { message: error.message };
901+
}
902+
903+
async function byokCredentialProfileHttpError(
904+
response: Response,
905+
): Promise<ByokCredentialProfileHttpError> {
906+
const fallback = `Failed to save BYOK credential (${response.status})`;
907+
try {
908+
const payload = await response.json() as unknown;
909+
if (
910+
payload
911+
&& typeof payload === 'object'
912+
&& 'error' in payload
913+
&& payload.error
914+
&& typeof payload.error === 'object'
915+
) {
916+
const message = 'message' in payload.error
917+
&& typeof payload.error.message === 'string'
918+
? payload.error.message.trim()
919+
: '';
920+
const code = 'code' in payload.error
921+
&& typeof payload.error.code === 'string'
922+
? payload.error.code.trim()
923+
: '';
924+
if (message) {
925+
return new ByokCredentialProfileHttpError(
926+
response.status,
927+
message,
928+
code || undefined,
929+
);
930+
}
931+
}
932+
} catch {
933+
// Keep the status-bearing fallback when the daemon/proxy body is invalid.
934+
}
935+
return new ByokCredentialProfileHttpError(response.status, fallback);
936+
}
937+
851938
export async function persistByokCredentialProfileToDaemon(
852939
input: UpsertByokCredentialProfileRequest,
853940
): Promise<ByokCredentialProfile> {
854-
const response = await fetch('/api/byok/profiles', {
855-
method: 'POST',
856-
headers: { 'content-type': 'application/json' },
857-
body: JSON.stringify(input),
858-
});
941+
let response: Response;
942+
try {
943+
response = await fetch('/api/byok/profiles', {
944+
method: 'POST',
945+
headers: { 'content-type': 'application/json' },
946+
body: JSON.stringify(input),
947+
});
948+
} catch (error) {
949+
const message = error instanceof Error && error.message.trim()
950+
? error.message
951+
: 'Failed to reach the local daemon while saving the BYOK credential.';
952+
throw new ByokCredentialProfileNetworkError(message, error);
953+
}
859954
if (!response.ok) {
860-
throw new Error(`Failed to save BYOK credential (${response.status})`);
955+
throw await byokCredentialProfileHttpError(response);
861956
}
862957
const payload = await response.json() as { profile?: ByokCredentialProfile };
863958
if (!payload.profile?.id || !payload.profile.configured) {

apps/web/tests/components/SettingsDialog.execution.test.tsx

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,7 @@ import { reconcileAmrProfileEnv } from '../../src/components/SettingsDialog';
112112
import { providerModelsCacheKey } from '../../src/components/providerModelsCache';
113113
import { I18nProvider } from '../../src/i18n';
114114
import { LOCALES } from '../../src/i18n/types';
115+
import { ByokCredentialProfileHttpError } from '../../src/state/config';
115116
import { MAX_MAX_TOKENS, MIN_MAX_TOKENS } from '../../src/state/maxTokens';
116117
import type {
117118
AgentInfo,
@@ -2380,6 +2381,60 @@ describe('SettingsDialog execution settings BYOK interactions', () => {
23802381
);
23812382
});
23822383

2384+
it('reports secure profile persistence failures with stable BYOK telemetry', async () => {
2385+
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
2386+
const url = input.toString();
2387+
if (url === '/api/memory') {
2388+
return new Response(
2389+
JSON.stringify({ enabled: true, memories: [], extraction: null }),
2390+
{ status: 200, headers: { 'content-type': 'application/json' } },
2391+
);
2392+
}
2393+
expect(url).toBe('/api/test/connection');
2394+
return new Response(
2395+
JSON.stringify({
2396+
ok: true,
2397+
kind: 'ok',
2398+
latencyMs: 20,
2399+
model: 'claude-sonnet-4-5',
2400+
sample: 'pong',
2401+
}),
2402+
{ status: 200, headers: { 'content-type': 'application/json' } },
2403+
);
2404+
});
2405+
vi.stubGlobal('fetch', fetchMock);
2406+
2407+
const { onPersistByokCredential } = renderSettingsDialog({
2408+
apiKey: 'sk-ant-test-provider',
2409+
});
2410+
onPersistByokCredential.mockRejectedValueOnce(
2411+
new ByokCredentialProfileHttpError(
2412+
400,
2413+
'Invalid secure profile',
2414+
'VALIDATION_FAILED',
2415+
),
2416+
);
2417+
2418+
fireEvent.click(screen.getByRole('button', { name: 'Test' }));
2419+
2420+
await waitFor(() => {
2421+
expect(analyticsTrackMock).toHaveBeenCalledWith(
2422+
'settings_byok_test_result',
2423+
expect.objectContaining({
2424+
page_name: 'settings',
2425+
area: 'execution_model',
2426+
provider_id: 'anthropic',
2427+
result: 'failed',
2428+
error_code: 'VALIDATION_FAILED',
2429+
error_kind: 'unknown',
2430+
field_missing: 'none',
2431+
success_after_action: false,
2432+
}),
2433+
undefined,
2434+
);
2435+
});
2436+
});
2437+
23832438
it('renders invalid Base URL test failures on the Base URL field', async () => {
23842439
const fetchMock = vi.fn(async (input: RequestInfo | URL) => {
23852440
const url = input.toString();

0 commit comments

Comments
 (0)