diff --git a/app/components/ToastProvider.tsx b/app/components/ToastProvider.tsx index 9c4df532..361281d3 100644 --- a/app/components/ToastProvider.tsx +++ b/app/components/ToastProvider.tsx @@ -9,9 +9,10 @@ import { X } from 'lucide-react'; import React, { useRef } from 'react'; import IconButton from '~/components/IconButton'; import cn from '~/utils/cn'; +import { ToastData } from '~/utils/toast'; -interface ToastProps extends AriaToastProps { - state: ToastState; +interface ToastProps extends AriaToastProps { + state: ToastState; } function Toast({ state, ...props }: ToastProps) { @@ -22,26 +23,38 @@ function Toast({ state, ...props }: ToastProps) { ref, ); + const { content, type } = props.toast.content; + const isError = type === 'error'; + const isSuccess = type === 'success'; + return (
-
-
{props.toast.content}
+
+
{content}
@@ -50,7 +63,7 @@ function Toast({ state, ...props }: ToastProps) { } interface ToastRegionProps extends AriaToastRegionProps { - state: ToastState; + state: ToastState; } function ToastRegion({ state, ...props }: ToastRegionProps) { @@ -60,18 +73,18 @@ function ToastRegion({ state, ...props }: ToastRegionProps) { return (
{state.visibleToasts.map((toast) => ( - + ))}
); } export interface ToastProviderProps extends AriaToastRegionProps { - queue: ToastQueue; + queue: ToastQueue; } export default function ToastProvider({ queue, ...props }: ToastProviderProps) { diff --git a/app/routes/acls/acl-action.ts b/app/routes/acls/acl-action.ts index bd946aec..911bee49 100644 --- a/app/routes/acls/acl-action.ts +++ b/app/routes/acls/acl-action.ts @@ -1,11 +1,34 @@ import { data } from 'react-router'; -import ResponseError from '~/server/headscale/api/response-error'; +import { isApiError } from '~/server/headscale/api/error-client'; +// import ResponseError from '~/server/headscale/api/response-error'; // Unused import { Capabilities } from '~/server/web/roles'; import type { Route } from './+types/overview'; // We only check capabilities here and assume it is writable // If it isn't, it'll gracefully error anyways, since this means some // fishy client manipulation is happening. +interface DataWithResponseInit { + data: { + data?: { + message?: string; + }; + rawData?: string; + }; + init: { + status?: number; + statusText?: string; + }; +} + +function isDataWithResponseInit(error: unknown): error is DataWithResponseInit { + return ( + typeof error === 'object' && + error !== null && + 'data' in error && + 'init' in error + ); +} + export async function aclAction({ request, context }: Route.ActionArgs) { const session = await context.sessions.auth(request); const check = await context.sessions.check( @@ -36,73 +59,159 @@ export async function aclAction({ request, context }: Route.ActionArgs) { policy, updatedAt, }); - } catch (error) { - // This means Headscale returned a protobuf error to us - // It also means we 100% know this is in database mode - if (error instanceof ResponseError && error.responseObject?.message) { - const message = error.responseObject.message as string; - // This is stupid, refer to the link - // https://github.com/juanfont/headscale/blob/main/hscontrol/types/policy.go - if (message.includes('update is disabled')) { - // This means the policy is not writable - throw data('Policy is not writable', { status: 403 }); - } + // biome-ignore lint/suspicious/noExplicitAny: Error handling needs to catch all types + } catch (error: unknown) { + console.error('ACL Action Error:', error); + + // Handle data() throw objects (DataWithResponseInit) which aren't instanceof Response + // but have the structure: { data: { ... }, init: { status: 502, ... } } + if (isDataWithResponseInit(error)) { + const statusCode = error.init.status || 500; + const statusText = error.init.statusText || 'Error'; - // https://github.com/juanfont/headscale/blob/main/hscontrol/policy/v1/acls.go#L81 - if (message.includes('parsing hujson')) { - // This means the policy was invalid, return a 400 - // with the actual error message from Headscale - const cutIndex = message.indexOf('err: hujson:'); - const trimmed = - cutIndex > -1 - ? `Syntax error: ${message.slice(cutIndex + 12)}` - : message; - - return data( - { - success: false, - error: trimmed, - policy: undefined, - updatedAt: undefined, - }, - 400, - ); + // The internal error from Headscale + const internalData = error.data.data; + let message = internalData?.message; + + if (!message) { + // Fallback to raw data or stringified object + message = error.data?.rawData || JSON.stringify(error.data); } - if (message.includes('unmarshalling policy')) { - // This means the policy was invalid, return a 400 - // with the actual error message from Headscale - const cutIndex = message.indexOf('err:'); - const trimmed = - cutIndex > -1 - ? `Syntax error: ${message.slice(cutIndex + 5)}` - : message; - - return data( - { - success: false, - error: trimmed, - policy: undefined, - updatedAt: undefined, - }, - 400, - ); + // Clean up common prefixes if present + if (typeof message === 'string') { + if (message.includes('setting policy:')) { + message = message.replace('setting policy:', '').trim(); + } + if (message.includes('parsing policy:')) { + message = message.replace('parsing policy:', '').trim(); + } } - if (message.includes('empty policy')) { - return data( - { - success: false, - error: 'Policy error: Supplied policy was empty', - policy: undefined, - updatedAt: undefined, - }, - 400, - ); + return data( + { + success: false, + error: `${message}\n\nStatus: ${statusCode} ${statusText}`, + policy: undefined, + updatedAt: undefined, + }, + // We return 200 or 400 to the UI so it renders the page with the error + // instead of triggering an ErrorBoundary + 400, + ); + } + + // This means Headscale returned a protobuf error to us + // It also means we 100% know this is in database mode + if (error instanceof Response) { + try { + const payload = await error.json(); + console.error('ACL Action Payload:', payload); + + if (isApiError(payload)) { + let message = + (payload.data?.message as string) || + payload.rawData || + 'Unknown error'; + + if (typeof message === 'object') { + message = JSON.stringify(message); + } + + // This is stupid, refer to the link + // https://github.com/juanfont/headscale/blob/main/hscontrol/types/policy.go + if (message.includes('update is disabled')) { + // This means the policy is not writable + return data( + { + success: false, + error: 'Policy is not writable (File mode enabled)', + policy: undefined, + updatedAt: undefined, + }, + 403, + ); + } + + // https://github.com/juanfont/headscale/blob/main/hscontrol/policy/v1/acls.go#L81 + if (message.includes('parsing hujson')) { + // This means the policy was invalid, return a 400 + // with the actual error message from Headscale + const cutIndex = message.indexOf('err: hujson:'); + const trimmed = + cutIndex > -1 + ? `Syntax error: ${message.slice(cutIndex + 12)}` + : message; + + return data( + { + success: false, + error: trimmed, + policy: undefined, + updatedAt: undefined, + }, + 400, + ); + } + + if (message.includes('unmarshalling policy')) { + // This means the policy was invalid, return a 400 + // with the actual error message from Headscale + const cutIndex = message.indexOf('err:'); + const trimmed = + cutIndex > -1 + ? `Syntax error: ${message.slice(cutIndex + 5)}` + : message; + + return data( + { + success: false, + error: trimmed, + policy: undefined, + updatedAt: undefined, + }, + 400, + ); + } + + if (message.includes('empty policy')) { + return data( + { + success: false, + error: 'Policy error: Supplied policy was empty', + policy: undefined, + updatedAt: undefined, + }, + 400, + ); + } + + // Return the raw error if no specific match + return data( + { + success: false, + error: message, + policy: undefined, + updatedAt: undefined, + }, + payload.statusCode, + ); + } + } catch (e) { + console.error('Failed to parse error response:', e); } } - // Otherwise, this is a Headscale error that we can just propagate. - throw error; + // Otherwise, catch generic errors and return them to the UI + // instead of throwing (which triggers ErrorBoundary). + return data( + { + success: false, + error: error instanceof Error ? error.message : String(error), + policy: undefined, + updatedAt: undefined, + }, + 500, + ); } } diff --git a/app/routes/acls/acl-loader.ts b/app/routes/acls/acl-loader.ts index e8c7bea0..4b68abbd 100644 --- a/app/routes/acls/acl-loader.ts +++ b/app/routes/acls/acl-loader.ts @@ -1,4 +1,5 @@ import { data } from 'react-router'; +import { isApiError } from '~/server/headscale/api/error-client'; import ResponseError from '~/server/headscale/api/response-error'; import { Capabilities } from '~/server/web/roles'; import type { Route } from './+types/overview'; @@ -39,18 +40,24 @@ export async function aclLoader({ request, context }: Route.LoaderArgs) { } catch (error) { // This means Headscale returned a protobuf error to us // It also means we 100% know this is in database mode - if (error instanceof ResponseError && error.responseObject?.message) { - const message = error.responseObject.message as string; - // This is stupid, refer to the link - // https://github.com/juanfont/headscale/blob/main/hscontrol/types/policy.go - if (message.includes('acl policy not found')) { - // This means the policy has never been initiated, and we can - // write to it to get it started or ignore it. - flags.policy = ''; // Start with an empty policy - flags.writable = true; - } + if (error instanceof Response) { + const payload = await error.json(); + if (isApiError(payload)) { + const message = + (payload.data?.message as string) || + payload.rawData || + 'Unknown error'; - return flags; + // This is stupid, refer to the link + // https://github.com/juanfont/headscale/blob/main/hscontrol/types/policy.go + if (message.includes('acl policy not found')) { + // This means the policy has never been initiated, and we can + // write to it to get it started or ignore it. + flags.policy = ''; // Start with an empty policy + flags.writable = true; + return flags; + } + } } // Otherwise, this is a Headscale error that we can just propagate. diff --git a/app/routes/acls/overview.tsx b/app/routes/acls/overview.tsx index 94b62ee5..98ee9cc0 100644 --- a/app/routes/acls/overview.tsx +++ b/app/routes/acls/overview.tsx @@ -32,13 +32,17 @@ export default function Page({ useEffect(() => { if (!fetcher.data) { - // No data yet, return return; } if (fetcher.data.success === true) { - toast('Updated policy'); + toast('ACL Policy updated', { type: 'success' }); revalidate(); + } else if (fetcher.data.error) { + toast(fetcher.data.error, { + type: 'error', + timeout: 1_000 * 60 * 60 * 24, // 24 hours (effectively infinite) + }); } }, [fetcher.data]); @@ -78,15 +82,7 @@ export default function Page({ .

- {fetcher.data?.error !== undefined ? ( - - {fetcher.data.error.split(':').slice(1).join(': ') ?? - 'An unknown error occurred while trying to update the ACL policy.'} - - ) : undefined} + ({ +export interface ToastData { + content: React.ReactNode; + type: 'success' | 'error' | 'default'; +} + +const toastQueue = new ToastQueue({ maxVisibleToasts: 7, }); @@ -9,6 +14,20 @@ export function useToastQueue() { return toastQueue; } -export default function toast(content: React.ReactNode, duration = 3000) { - return toastQueue.add(content, { timeout: duration }); +export type ToastOptions = { + type?: 'success' | 'error' | 'default'; + timeout?: number; +}; + +export default function toast( + content: React.ReactNode, + options?: ToastOptions | number, +) { + const timeout = typeof options === 'number' ? options : options?.timeout; + const type = typeof options === 'object' ? options.type : 'default'; + + return toastQueue.add( + { content, type: type ?? 'default' }, + { timeout: timeout ?? 3000 }, + ); }