Skip to content

Commit e95afaa

Browse files
claude[bot]github-actions[bot]claude
authored
types: improve type safety in error handling — replace as-casts with type guards (#4638)
`catch` bindings and query `error` fields are `unknown` — the thrower picks the shape. Nine sites asserted one anyway (`(error as Error).message`, `error as Error`, `error as { status?: number }`), so a thrown string gave `undefined` where a string was expected and a thrown `null` crashed the handler that was meant to report it. Replaces those assertions with structural narrowing: `getErrorMessage` in web's `errorHandling.ts`, `toError`/`messageOf` local to mobile's WebSocketManager, and inline guards where a single site reads a single field. The one behavior difference is on inputs that already misbehaved: a thrown `null` now falls through to the retry path instead of throwing a TypeError inside the catch block. Co-authored-by: claude[bot] <41898282+claude[bot]@users.noreply.github.com> Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
1 parent 09b4e64 commit e95afaa

9 files changed

Lines changed: 77 additions & 30 deletions

File tree

mobile/src/components/graph_editor/PropertyField.tsx

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -969,8 +969,11 @@ const JSONWidget: React.FC<{
969969
colors: ThemeColors;
970970
}> = ({ prop, value, onChange, colors }) => {
971971
const dataStr =
972-
typeof value === "object" && value !== null
973-
? (value as Record<string, unknown>).data as string | undefined
972+
typeof value === "object" &&
973+
value !== null &&
974+
"data" in value &&
975+
typeof value.data === "string"
976+
? value.data
974977
: typeof value === "string"
975978
? value
976979
: undefined;
@@ -983,7 +986,7 @@ const JSONWidget: React.FC<{
983986
JSON.parse(localValue);
984987
setError(null);
985988
} catch (e) {
986-
setError((e as Error).message);
989+
setError(e instanceof Error ? e.message : "Invalid JSON");
987990
}
988991
onChange({ type: "json", data: localValue });
989992
}, [localValue, onChange]);

mobile/src/hooks/useApplications.ts

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,14 @@ import {
3333
import { trpc } from '../trpc/client';
3434
import type { Workflow } from '../types/workflow';
3535

36+
/** tRPC types its `error` as a plain interface, not an `Error` subtype. */
37+
function toError(error: { message: string } | null): Error | null {
38+
if (!error) {
39+
return null;
40+
}
41+
return error instanceof Error ? error : new Error(error.message);
42+
}
43+
3644
export const applicationKeys = {
3745
all: ['applications'] as const,
3846
list: (projectId?: string): QueryKey => [
@@ -187,10 +195,6 @@ export function useApplicationApp(id: string | undefined): ApplicationApp {
187195
application.isLoading ||
188196
release.isLoading ||
189197
(Boolean(workflowId) && !pinned && liveWorkflow.isLoading),
190-
error:
191-
(application.error as Error | null) ??
192-
(release.error as Error | null) ??
193-
(liveWorkflow.error as Error | null) ??
194-
null,
198+
error: application.error ?? release.error ?? toError(liveWorkflow.error),
195199
};
196200
}

mobile/src/queryClient.ts

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -31,12 +31,17 @@ function httpStatusFromError(error: unknown): number | undefined {
3131
if (typeof error !== 'object' || error === null) {
3232
return undefined;
3333
}
34-
const e = error as { status?: unknown; data?: { httpStatus?: unknown } };
35-
if (typeof e.status === 'number') {
36-
return e.status;
34+
if ('status' in error && typeof error.status === 'number') {
35+
return error.status;
3736
}
38-
if (typeof e.data?.httpStatus === 'number') {
39-
return e.data.httpStatus;
37+
if (
38+
'data' in error &&
39+
typeof error.data === 'object' &&
40+
error.data !== null &&
41+
'httpStatus' in error.data &&
42+
typeof error.data.httpStatus === 'number'
43+
) {
44+
return error.data.httpStatus;
4045
}
4146
return undefined;
4247
}

mobile/src/services/WebSocketManager.ts

Lines changed: 27 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,24 @@ import { isAppForeground, subscribeAppLifecycle } from '../hooks/useAppLifecycle
1818

1919
type WebSocketMessage = { type: string };
2020

21+
/** The message an unknown thrown value or error event carries, or `fallback`. */
22+
function messageOf(value: unknown, fallback: string): string {
23+
return typeof value === 'object' &&
24+
value !== null &&
25+
'message' in value &&
26+
typeof value.message === 'string' &&
27+
value.message
28+
? value.message
29+
: fallback;
30+
}
31+
32+
/** A `catch` binding is `unknown` — narrow it rather than assert it. */
33+
function toError(thrown: unknown, fallback: string): Error {
34+
return thrown instanceof Error
35+
? thrown
36+
: new Error(messageOf(thrown, fallback));
37+
}
38+
2139
interface WebSocketCallbacks {
2240
onStateChange?: (state: ConnectionState, previousState: ConnectionState) => void;
2341
onMessage?: (data: WebSocketMessageData) => void;
@@ -250,7 +268,7 @@ export class WebSocketManager {
250268
this.ws!.send(encoded);
251269
} catch (error) {
252270
console.error('Failed to send message:', error);
253-
this.callbacks.onError?.(error as Error);
271+
this.callbacks.onError?.(toError(error, 'Failed to send message'));
254272
throw error;
255273
}
256274
}
@@ -260,7 +278,7 @@ export class WebSocketManager {
260278

261279
this.ws.onopen = () => this.handleOpen();
262280
this.ws.onmessage = (event) => this.handleMessage(event);
263-
this.ws.onerror = (event) => this.handleError(event as { message?: string });
281+
this.ws.onerror = (event) => this.handleError(event);
264282
this.ws.onclose = (event) => this.handleClose(event);
265283
}
266284

@@ -319,7 +337,7 @@ export class WebSocketManager {
319337
this.callbacks.onMessage?.(data as WebSocketMessageData);
320338
} catch (error) {
321339
console.error('Failed to process message:', error);
322-
this.callbacks.onError?.(error as Error);
340+
this.callbacks.onError?.(toError(error, 'Failed to process message'));
323341
}
324342
}
325343

@@ -354,9 +372,11 @@ export class WebSocketManager {
354372
});
355373
}
356374

357-
private handleError(event: { message?: string }): void {
375+
private handleError(event: unknown): void {
358376
console.error('WebSocket error:', event);
359-
this.callbacks.onError?.(new Error(event.message || 'WebSocket error occurred'));
377+
this.callbacks.onError?.(
378+
new Error(messageOf(event, 'WebSocket error occurred'))
379+
);
360380
}
361381

362382
private handleClose(event: WebSocketCloseEvent): void {
@@ -494,7 +514,7 @@ export class WebSocketManager {
494514
this.setupEventHandlers();
495515
this.startConnectionTimeout();
496516
} catch (error) {
497-
this.handleConnectionError(error as Error);
517+
this.handleConnectionError(toError(error, 'Failed to open WebSocket'));
498518
reject(error);
499519
}
500520
});
@@ -663,7 +683,7 @@ export class WebSocketManager {
663683
this.send(message);
664684
} catch (error) {
665685
console.error('Failed to send queued message:', error);
666-
this.callbacks.onError?.(error as Error);
686+
this.callbacks.onError?.(toError(error, 'Failed to send queued message'));
667687
}
668688
}
669689
}

web/src/components/script/ScriptLineRow.tsx

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import {
3939
import { voiceLine } from "../../stores/script/scriptVoicing";
4040
import { useAssetStore } from "../../stores/AssetStore";
4141
import { getAssetUrl } from "../../utils/assetHelpers";
42+
import { getErrorMessage } from "../../utils/errorHandling";
4243
import ScriptTakeGallery from "./ScriptTakeGallery";
4344

4445
/**
@@ -195,7 +196,7 @@ const ScriptLineRow = ({
195196
try {
196197
await voiceLine(scriptId, line.id);
197198
} catch (error) {
198-
setVoiceError((error as Error).message ?? "Voicing failed");
199+
setVoiceError(getErrorMessage(error, "Voicing failed"));
199200
}
200201
}, [scriptId, line.id]);
201202

web/src/hooks/script/useScriptServerSync.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import {
1717
type ScriptDraft,
1818
type ScriptSaveStatus
1919
} from "../../stores/script/ScriptStore";
20+
import { getErrorMessage } from "../../utils/errorHandling";
2021

2122
const AUTOSAVE_DEBOUNCE_MS = 750;
2223
const RETRY_DELAY_MS = 5_000;
@@ -43,9 +44,7 @@ const responseToScript = (
4344
};
4445

4546
const isNotFound = (error: unknown): boolean =>
46-
!!error &&
47-
typeof error === "object" &&
48-
/not found/i.test((error as Error).message ?? "");
47+
/not found/i.test(getErrorMessage(error));
4948

5049
export const useScriptServerSync = (scriptId: string): void => {
5150
const utils = trpc.useUtils();
@@ -162,7 +161,7 @@ export const useScriptServerSync = (scriptId: string): void => {
162161
store.getState().setSaveStatus(scriptId, "error");
163162
return;
164163
}
165-
if (/modified since last read/i.test((error as Error).message ?? "")) {
164+
if (/modified since last read/i.test(getErrorMessage(error))) {
166165
// Set "reloaded" only after the server copy is applied, not before.
167166
await load("reloaded");
168167
} else {

web/src/hooks/storyboard/useStoryboardServerSync.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import {
1616
type StoryboardBoard
1717
} from "../../stores/storyboard/StoryboardStore";
1818
import type { Screenplay, Shot } from "@nodetool-ai/protocol";
19+
import { getErrorMessage } from "../../utils/errorHandling";
1920

2021
const AUTOSAVE_DEBOUNCE_MS = 750;
2122
const RETRY_DELAY_MS = 5_000;
@@ -61,9 +62,7 @@ const responseToBoard = (
6162
};
6263

6364
const isNotFound = (error: unknown): boolean =>
64-
!!error &&
65-
typeof error === "object" &&
66-
/not found/i.test((error as Error).message ?? "");
65+
/not found/i.test(getErrorMessage(error));
6766

6867
export const useStoryboardServerSync = (boardId: string): void => {
6968
const utils = trpc.useUtils();
@@ -139,7 +138,7 @@ export const useStoryboardServerSync = (boardId: string): void => {
139138
} catch (error) {
140139
if (disposed) return;
141140
console.error("Storyboard autosave failed", error);
142-
if (/modified since last read/i.test((error as Error).message ?? "")) {
141+
if (/modified since last read/i.test(getErrorMessage(error))) {
143142
// CAS conflict: the server copy wins.
144143
await load();
145144
} else {

web/src/stores/AssetStore.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -144,7 +144,13 @@ const uploadAsset = async (
144144
emitUploadProgress(onUploadProgress, total, total);
145145
return normalizeAssetUrls(data as Asset);
146146
} catch (error) {
147-
const statusCode = (error as { status?: number })?.status;
147+
const statusCode =
148+
typeof error === "object" &&
149+
error !== null &&
150+
"status" in error &&
151+
typeof error.status === "number"
152+
? error.status
153+
: undefined;
148154
const normalizedError =
149155
error instanceof DOMException && error.name === "AbortError"
150156
? createErrorMessage(error, "Asset upload was cancelled")

web/src/utils/errorHandling.ts

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,16 @@ export class AppError extends Error {
55
}
66
}
77

8+
const hasErrorMessage = (error: unknown): error is { message: string } =>
9+
typeof error === "object" &&
10+
error !== null &&
11+
"message" in error &&
12+
typeof error.message === "string";
13+
14+
/** The message of a thrown value, or `fallback` when it carries none. */
15+
export const getErrorMessage = (error: unknown, fallback = ""): string =>
16+
hasErrorMessage(error) ? error.message : fallback;
17+
818
export const createErrorMessage = (
919
error: unknown,
1020
defaultMessage: string

0 commit comments

Comments
 (0)