Skip to content

Commit f125767

Browse files
committed
refactor(ui): name the failure policy instead of an empty catch
The 21 empty catch blocks each carried a sentence saying why an error was swallowed. Under the allowlist that sentence is allowed to stay — the body of an empty block is one of the four kinds of comment the rule keeps — but the comment was never the best form for it. None of these needed a comment. They needed a name. utils/failure.ts holds four, each documented with when it is the right one: ignoreFailure and nullOnFailure for .catch(), attempt for a synchronous call that may not be there at all, and succeeded for a call whose error is already recorded somewhere the screen reads. Most sites did not even need those. The failure had an answer the code was not stating: parseMessage returns null, because a frame that is not JSON is ordinary terminal output; loadConfig falls back in the expression, so the default is visible in the data flow; markWelcomeSeen returns whether it stuck, mirroring hasSeenWelcome; listRecordings reads each sidecar through a helper returning RecordingMeta | null, so a corrupt one is skipped by an if; and four react-query call sites used mutateAsync inside a try that discarded the rejection, where mutate() reports through the mutation's own state. ForgotPassword keeps its reason in the code, as a name: const silenceToPreventAccountEnumeration = ignoreFailure; await recoverPassword({ ... }).catch(silenceToPreventAccountEnumeration); Two behaviours are better than before rather than merely equivalent: Chatwoot records the identity only when setUser actually succeeded, and the settings toggles clear their busy flag through onSettled instead of a finally that ran whether or not the mutation was still relevant. This is the same change as on the total-ban branch, where it removes 21 lint suppressions. Here it removes 21 comments the rule would have allowed, which is the more interesting result: the allowance was carrying code that wanted rewriting.
1 parent 9eb5c4e commit f125767

17 files changed

Lines changed: 164 additions & 140 deletions

ui/apps/console/src/components/common/ConfirmDialog.tsx

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import { Button, type ButtonVariant } from "@shellhub/design-system/primitives";
44
import { cn } from "@shellhub/design-system/cn";
55
import { useResetOnOpen } from "@/hooks/useResetOnOpen";
66
import BaseDialog from "./BaseDialog";
7+
import { ignoreFailure } from "@/utils/failure";
78

89
interface ConfirmDialogProps {
910
/** Controls open/close state. */
@@ -82,15 +83,11 @@ export default function ConfirmDialog({
8283
setConfirming(false);
8384
});
8485

85-
const handleConfirm = async () => {
86+
const handleConfirm = () => {
8687
setConfirming(true);
87-
try {
88-
await onConfirm();
89-
} catch {
90-
// Errors are not surfaced here. Consumers manage their own error state
91-
} finally {
92-
setConfirming(false);
93-
}
88+
void Promise.resolve(onConfirm())
89+
.catch(ignoreFailure)
90+
.finally(() => setConfirming(false));
9491
};
9592

9693
const buttonVariant = VARIANT_BUTTON[variant];

ui/apps/console/src/components/common/CreateNamespace.tsx

Lines changed: 10 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ import {
2323
GithubIcon,
2424
Spinner,
2525
} from "@shellhub/design-system/primitives";
26+
import { nullOnFailure } from "@/utils/failure";
2627

2728
export function NamespaceCreateForm() {
2829
const [name, setName] = useState("");
@@ -113,17 +114,15 @@ export function CommunityInstructions() {
113114

114115
useEffect(() => {
115116
const check = async () => {
116-
try {
117-
const { data } = await getNamespaces({
118-
query: { page: 1, per_page: 1 },
119-
throwOnError: true,
120-
});
121-
if (data.length > 0) {
122-
setReady(true);
123-
setTenantId(data[0].tenant_id);
124-
}
125-
} catch {
126-
// ignore
117+
const result = await getNamespaces({
118+
query: { page: 1, per_page: 1 },
119+
throwOnError: true,
120+
}).catch(nullOnFailure);
121+
122+
const first = result?.data[0];
123+
if (first) {
124+
setReady(true);
125+
setTenantId(first.tenant_id);
127126
}
128127
};
129128
const interval = setInterval(() => void check(), 5000);

ui/apps/console/src/components/terminal/TerminalInstance.tsx

Lines changed: 2 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ import {
2727
} from "./terminalErrors";
2828
import SSHApproval from "@/pages/SSHApproval";
2929
import { cn } from "@shellhub/design-system/cn";
30+
import { attempt } from "@/utils/failure";
3031

3132
interface TerminalInstanceProps {
3233
session: TerminalSession;
@@ -137,11 +138,7 @@ export default function TerminalInstance({
137138
if (!containerRef.current) return;
138139
term.open(containerRef.current);
139140

140-
try {
141-
term.loadAddon(new WebglAddon());
142-
} catch {
143-
// DOM renderer fallback
144-
}
141+
attempt(() => term.loadAddon(new WebglAddon()));
145142

146143
fitAddon.fit();
147144
const { cols, rows } = term;

ui/apps/console/src/components/terminal/terminalErrors.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -175,7 +175,7 @@ export function parseMessage(
175175
};
176176
}
177177
} catch {
178-
// Not JSON — regular text frame
178+
return null;
179179
}
180180
return null;
181181
}

ui/apps/console/src/env.ts

Lines changed: 9 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
import { nullOnFailure } from "@/utils/failure";
2+
13
export type Edition = "community" | "enterprise" | "cloud";
24

35
export interface ClientConfig {
@@ -35,17 +37,13 @@ export async function loadConfig(): Promise<ClientConfig> {
3537
if (inflight) return inflight;
3638

3739
inflight = (async () => {
38-
try {
39-
const res = await fetch("/config.json");
40-
cached = {
41-
...defaultConfig,
42-
...((await res.json()) as Partial<ClientConfig>),
43-
};
44-
} catch {
45-
// leave cached as defaultConfig so future calls can retry
46-
} finally {
47-
inflight = null;
48-
}
40+
const fetched = await fetch("/config.json")
41+
.then((res) => res.json() as Promise<Partial<ClientConfig>>)
42+
.catch(nullOnFailure);
43+
44+
cached = fetched ? { ...defaultConfig, ...fetched } : defaultConfig;
45+
inflight = null;
46+
4947
return cached;
5048
})();
5149

ui/apps/console/src/hooks/chatwootRuntime.ts

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,4 @@
1+
import { attempt } from "@/utils/failure";
12
/**
23
* Module-level lifecycle for the Chatwoot SDK. Lives outside the React tree
34
* so `authStore.logout()` can tear it down without going through a hook, and
@@ -156,12 +157,10 @@ export function tearDownChatwoot(
156157
clearWatchdog();
157158
detachReadyListener();
158159

159-
try {
160+
attempt(() => {
160161
window.$chatwoot?.toggle("close");
161162
window.$chatwoot?.reset();
162-
} catch {
163-
// Widget mid-bootstrap — nothing to close.
164-
}
163+
});
165164

166165
document.getElementById(SCRIPT_ID)?.remove();
167166
document.querySelectorAll(SDK_DOM_SELECTORS).forEach((node) => node.remove());

ui/apps/console/src/hooks/useChatwoot.ts

Lines changed: 8 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ import {
1818
injectChatwootScript,
1919
subscribeChatwootState,
2020
} from "@/hooks/chatwootRuntime";
21+
import { attempt } from "@/utils/failure";
2122

2223
export type ChatwootStatus =
2324
"non-cloud" | "unavailable" | "no-subscription" | "loading" | "ready";
@@ -101,16 +102,15 @@ export function useChatwoot(): ChatwootHandle {
101102
].join("|");
102103
if (lastIdentityRef.current === key) return;
103104

104-
try {
105+
const identified = attempt(() => {
105106
window.$chatwoot?.setUser(userId, {
106107
email: userEmail ?? undefined,
107108
name: userName ?? undefined,
108109
identifier_hash: identifier,
109110
});
110-
lastIdentityRef.current = key;
111-
} catch {
112-
// Widget reset between effect setup and call — next change retries.
113-
}
111+
});
112+
113+
if (identified) lastIdentityRef.current = key;
114114
}, [widgetReady, userId, userEmail, userName, tenant, identifier]);
115115

116116
useEffect(() => {
@@ -120,15 +120,13 @@ export function useChatwoot(): ChatwootHandle {
120120
const onMessage = () => {
121121
if (fired) return;
122122
fired = true;
123-
try {
123+
attempt(() => {
124124
window.$chatwoot?.setConversationCustomAttributes({
125125
namespace: namespaceName,
126126
tenant,
127127
domain: window.location.hostname,
128128
});
129-
} catch {
130-
// Ignore — Chatwoot may not expose the API in older builds.
131-
}
129+
});
132130
};
133131

134132
window.addEventListener("chatwoot:on-message", onMessage);
@@ -137,11 +135,7 @@ export function useChatwoot(): ChatwootHandle {
137135

138136
const openWidget = useCallback(() => {
139137
if (!widgetReady) return;
140-
try {
141-
window.$chatwoot?.toggle("open");
142-
} catch {
143-
// Widget not yet attached — no-op.
144-
}
138+
attempt(() => window.$chatwoot?.toggle("open"));
145139
}, [widgetReady]);
146140

147141
let status: ChatwootStatus;

ui/apps/console/src/pages/ForgotPassword.tsx

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ import {
1414
type ForgotPasswordFormValues,
1515
} from "./setup/forgotPasswordResolver";
1616
import LoginLayoutCard from "@/components/layout/LoginLayoutCard";
17+
import { ignoreFailure } from "@/utils/failure";
18+
19+
const silenceToPreventAccountEnumeration = ignoreFailure;
1720

1821
export default function ForgotPassword() {
1922
const [loading, setLoading] = useState(false);
@@ -28,17 +31,14 @@ export default function ForgotPassword() {
2831

2932
const onSubmit = async (values: ForgotPasswordFormValues) => {
3033
setLoading(true);
31-
try {
32-
await recoverPassword({
33-
body: { username: values.account },
34-
throwOnError: true,
35-
});
36-
} catch {
37-
// Silently ignore to prevent user enumeration.
38-
} finally {
39-
setLoading(false);
40-
setSent(true);
41-
}
34+
35+
await recoverPassword({
36+
body: { username: values.account },
37+
throwOnError: true,
38+
}).catch(silenceToPreventAccountEnumeration);
39+
40+
setLoading(false);
41+
setSent(true);
4242
};
4343

4444
return (

ui/apps/console/src/pages/MfaResetRequest.tsx

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import { useAuthStore } from "../stores/authStore";
77
import { useMfaResetStore } from "../stores/mfaResetStore";
88
import AuthFooterLinks from "../components/common/AuthFooterLinks";
99
import LoginLayoutCard from "@/components/layout/LoginLayoutCard";
10+
import { succeeded } from "@/utils/failure";
1011

1112
export default function MfaResetRequest() {
1213
const { user, username, mfaToken } = useAuthStore();
@@ -32,11 +33,8 @@ export default function MfaResetRequest() {
3233
}
3334

3435
const onSubmit = async () => {
35-
try {
36-
await requestMfaReset(identifier);
36+
if (await succeeded(requestMfaReset(identifier))) {
3737
void navigate("/mfa-reset-verify");
38-
} catch {
39-
// Error is set in store
4038
}
4139
};
4240

ui/apps/console/src/pages/Settings.tsx

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -353,11 +353,11 @@ export default function Settings() {
353353
const sshLegacyAllowed = settings?.ssh_legacy_allowed ?? false;
354354
const banner = settings?.connection_announcement ?? "";
355355

356-
const handleToggleRecord = async () => {
356+
const handleToggleRecord = () => {
357357
if (!tenantId || togglingRecord) return;
358358
setTogglingRecord(true);
359-
try {
360-
await editNs.mutateAsync({
359+
editNs.mutate(
360+
{
361361
path: { tenant: tenantId },
362362
body: {
363363
settings: {
@@ -366,27 +366,21 @@ export default function Settings() {
366366
ssh_access_mode: sshAccessMode,
367367
},
368368
},
369-
});
370-
} catch {
371-
/* state didn't change */
372-
} finally {
373-
setTogglingRecord(false);
374-
}
369+
},
370+
{ onSettled: () => setTogglingRecord(false) },
371+
);
375372
};
376373

377-
const handleSetAccessMode = async (mode: "legacy" | "identity") => {
374+
const handleSetAccessMode = (mode: "legacy" | "identity") => {
378375
if (!tenantId || switchingAccessMode || mode === sshAccessMode) return;
379376
setSwitchingAccessMode(true);
380-
try {
381-
await setSshAccessMode.mutateAsync({
377+
setSshAccessMode.mutate(
378+
{
382379
path: { tenant: tenantId },
383380
body: { ssh_access_mode: mode },
384-
});
385-
} catch {
386-
/* state didn't change */
387-
} finally {
388-
setSwitchingAccessMode(false);
389-
}
381+
},
382+
{ onSettled: () => setSwitchingAccessMode(false) },
383+
);
390384
};
391385

392386
if (!ns) {
@@ -478,7 +472,7 @@ export default function Settings() {
478472
<button
479473
type="button"
480474
onClick={() => {
481-
if (sessionRecord) void handleToggleRecord();
475+
if (sessionRecord) handleToggleRecord();
482476
}}
483477
className={cn(
484478
"h-full px-2.5 text-2xs font-medium rounded transition-all duration-150",
@@ -492,7 +486,7 @@ export default function Settings() {
492486
<button
493487
type="button"
494488
onClick={() => {
495-
if (!sessionRecord) void handleToggleRecord();
489+
if (!sessionRecord) handleToggleRecord();
496490
}}
497491
className={cn(
498492
"h-full px-2.5 text-2xs font-medium rounded transition-all duration-150",
@@ -544,7 +538,7 @@ export default function Settings() {
544538
>
545539
<button
546540
type="button"
547-
onClick={() => void handleSetAccessMode("legacy")}
541+
onClick={() => handleSetAccessMode("legacy")}
548542
className={`h-full px-2.5 text-2xs font-medium rounded transition-all duration-150 ${
549543
sshAccessMode === "legacy"
550544
? "bg-hover-strong text-text-secondary border border-border-light"
@@ -555,7 +549,7 @@ export default function Settings() {
555549
</button>
556550
<button
557551
type="button"
558-
onClick={() => void handleSetAccessMode("identity")}
552+
onClick={() => handleSetAccessMode("identity")}
559553
className={`h-full px-2.5 text-2xs font-medium rounded transition-all duration-150 ${
560554
sshAccessMode === "identity"
561555
? "bg-primary/15 text-primary border border-primary/25"

0 commit comments

Comments
 (0)