Skip to content

Commit

Permalink
improve document error handling (#310)
Browse files Browse the repository at this point in the history
- add better error display and ability to navigate when a document errors unexpectedly
- refactor Alert to simplify interface
- add better behavior when document not found (navigates back to homepage with toast); clean-up invalid error handling

Closes #260
  • Loading branch information
cloverich authored Feb 3, 2025
1 parent 7b0ce48 commit 1105ff5
Show file tree
Hide file tree
Showing 7 changed files with 244 additions and 37 deletions.
56 changes: 52 additions & 4 deletions src/components/Alert.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { cva, type VariantProps } from "class-variance-authority";
import { AngryIcon, InfoIcon, OctagonIcon, TriangleIcon } from "lucide-react";
import * as React from "react";

import { cn } from "@udecode/cn";
Expand All @@ -11,6 +12,8 @@ const alertVariants = cva(
default: "bg-background text-foreground",
destructive:
"border-destructive/50 text-destructive dark:border-destructive [&>svg]:text-destructive",
warning: "border-warning/50 text-warning dark:border-warning",
error: "border-error/50 text-error dark:border-error",
},
},
defaultVariants: {
Expand All @@ -19,7 +22,51 @@ const alertVariants = cva(
},
);

const iconForVariant = {
default: InfoIcon,
destructive: OctagonIcon,
warning: TriangleIcon,
error: AngryIcon,
};

interface IAlert {
// variant: "default" | "destructive" | "warning" | "error";
title: string;
}

const Alert = React.forwardRef<
HTMLDivElement,
React.HTMLAttributes<HTMLDivElement> &
VariantProps<typeof alertVariants> &
IAlert
>(({ className, variant, title, children }, ref) => {
const Icon = iconForVariant[variant || "default"];
return (
<AlertBase ref={ref} variant={variant} className={className}>
<div className="flex">
{/* This worked ok to display Icon but layout wasn't great */}
{/* <div className="mr-2 h-6 w-6">
<Icon
strokeWidth={1}
className="mr-2 h-6 w-6"
size={32}
fontSize={32}
height={32}
width={32}
/>
</div> */}
<div>
<AlertTitle>{title}</AlertTitle>
<AlertDescription>{children}</AlertDescription>
</div>
</div>
</AlertBase>
);
});

Alert.displayName = "Alert";

const AlertBase = React.forwardRef<
HTMLDivElement,
React.HTMLAttributes<HTMLDivElement> & VariantProps<typeof alertVariants>
>(({ className, variant, ...props }, ref) => (
Expand All @@ -30,21 +77,22 @@ const Alert = React.forwardRef<
{...props}
/>
));
Alert.displayName = "Alert";
AlertBase.displayName = "AlertBase";

const AlertTitle = React.forwardRef<
HTMLParagraphElement,
React.HTMLAttributes<HTMLHeadingElement>
>(({ className, ...props }, ref) => (
<h5
<div
ref={ref}
className={cn(
"mb-1 mt-1 font-medium leading-none tracking-tight",
"mb-1 mt-1 text-lg font-medium leading-none tracking-tight",
className,
)}
{...props}
/>
));

AlertTitle.displayName = "AlertTitle";

const AlertDescription = React.forwardRef<
Expand All @@ -59,4 +107,4 @@ const AlertDescription = React.forwardRef<
));
AlertDescription.displayName = "AlertDescription";

export { Alert, AlertDescription, AlertTitle };
export { Alert, AlertBase, AlertDescription, AlertTitle };
4 changes: 4 additions & 0 deletions src/error.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,10 @@ interface State {
error: any;
}

interface Props {
children: React.ReactNode;
}

export default class ErrorBoundary extends React.Component<any, State> {
constructor(props: any) {
super(props);
Expand Down
134 changes: 134 additions & 0 deletions src/views/edit/EditorErrorBoundary.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,134 @@
import { ChevronLeftIcon, IconButton } from "evergreen-ui";
import React from "react";
import { NavigateFunction } from "react-router-dom";
import { Alert } from "../../components";
import Titlebar from "../../titlebar/macos";
import * as Base from "../layout";
import { Separator } from "./editor/components/Separator";

interface State {
hasError: boolean;
error: any;
}

interface Props {
children: React.ReactNode;
navigate: NavigateFunction;
documentId: string;
journal: string;
}

/**
* Wraps the editor in an error boundary that catches and displays errors in an editor-friendly way.
*/
export default class EditorErrorBoundary extends React.Component<Props, State> {
constructor(props: any) {
super(props);
this.state = { hasError: false, error: null };
}

static getDerivedStateFromError(error: any) {
return { hasError: true, error };
}

componentDidCatch(error: any, errorInfo: any) {
console.error(
"EditorErrorBoundary error boundary reached:",
error,
errorInfo,
);
}

renderError() {
const { errStr, stack } = decomposeError(this.state.error);

return (
<Base.EditorContainer>
<Titlebar>
<IconButton
backgroundColor="transparent"
border="none"
icon={ChevronLeftIcon}
className="drag-none"
onClick={() => this.props.navigate("/documents")}
marginRight={8}
>
Back to documents
</IconButton>
<Separator orientation="vertical" />
</Titlebar>

{/* This Ghost div is same height as titlebar, so pushes the main content below it -- necessary for the contents scrollbar to make sense */}
<Base.TitlebarSpacer />
<Base.ScrollContainer>
<ErrorContent
error={this.state.error}
journal={this.props.journal}
documentId={this.props.documentId}
/>
</Base.ScrollContainer>
</Base.EditorContainer>
);
}

render() {
if (this.state.hasError) {
return this.renderError();
}

return this.props.children;
}
}

export function decomposeError(error: any) {
let errStr: string = "Unable to parse error for display. Check console? :|";
let stack: string = "";
try {
if (error instanceof Error) {
errStr = error.message;
stack = error.stack || "";
} else if (typeof error === "string") {
errStr = error;
} else {
errStr = JSON.stringify(error, null, 2);
}
errStr = JSON.stringify(error, null, 2);
} catch (err) {
console.error("Error parsing error to string in top-level Error boundary");
}

return { errStr, stack };
}

export function ErrorContent({
error,
journal,
documentId,
}: {
error: any;
journal?: string;
documentId?: string;
}) {
const { errStr, stack } = decomposeError(error);

return (
<Alert.Alert
variant="error"
title="Unhandled Error"
className="overflow-x-auto"
>
<div>
<p>There was an error that crashed the editor</p>
{documentId && (
<p>
<span className="font-medium">Offending document:</span>{" "}
{journal || "Unknown Journal"}/{documentId}
.md
</p>
)}
{errStr && errStr != "{}" && <pre>{errStr}</pre>}
<pre>{stack}</pre>
</div>
</Alert.Alert>
);
}
40 changes: 29 additions & 11 deletions src/views/edit/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import Titlebar from "../../titlebar/macos";
import { useSearchStore } from "../documents/SearchStore";
import * as Base from "../layout";
import { EditableDocument } from "./EditableDocument";
import EditorErrorBoundary from "./EditorErrorBoundary";
import { EditorMode } from "./EditorMode";
import FrontMatter from "./FrontMatter";
import PlateContainer from "./PlateContainer";
Expand Down Expand Up @@ -42,8 +43,6 @@ const DocumentLoadingContainer = observer(() => {
const journalsStore = useJournals();
const { document: documentId } = useParams();

// todo: handle missing or invalid documentId; loadingError may be fine for this, but
// haven't done any UX / design thinking around it.
const {
document,
error: loadingError,
Expand All @@ -54,6 +53,8 @@ const DocumentLoadingContainer = observer(() => {
// the current document's journal if its archived
const [journals, setJournals] = useState<any>();

// Filter journals to non-archived ones, but must also add
// the current document's journal if its archived
useEffect(() => {
if (!document) return;

Expand All @@ -69,13 +70,24 @@ const DocumentLoadingContainer = observer(() => {
}, [document, loadingError]);

if (loadingError) {
return <EditLoadingComponent error={loadingError} />;
return (
<EditLoadingComponent
journal={document?.journal}
documentId={documentId}
error={loadingError}
/>
);
}

// `loading` acts as a break when navigating from one document to another, effectively
// resetting the state of the editor
if (!document || !journals || loading) {
return <EditLoadingComponent />;
return (
<EditLoadingComponent
journal={document?.journal}
documentId={documentId}
/>
);
}

return <DocumentEditView document={document} journals={journals} />;
Expand Down Expand Up @@ -128,13 +140,19 @@ const DocumentEditView = observer((props: DocumentEditProps) => {
selectedEditorMode={selectedViewMode}
setSelectedEditorMode={setSelectedViewMode}
>
<EditorInner
document={document}
selectedViewMode={selectedViewMode}
setSelectedViewMode={setSelectedViewMode}
journals={journals}
goBack={goBack}
/>
<EditorErrorBoundary
documentId={document.id}
journal={document.journal}
navigate={navigate}
>
<EditorInner
document={document}
selectedViewMode={selectedViewMode}
setSelectedViewMode={setSelectedViewMode}
journals={journals}
goBack={goBack}
/>
</EditorErrorBoundary>
</PlateContainer>
);
});
Expand Down
19 changes: 11 additions & 8 deletions src/views/edit/loading.tsx
Original file line number Diff line number Diff line change
@@ -1,12 +1,15 @@
import { ChevronLeftIcon, IconButton, Pane } from "evergreen-ui";
import { ChevronLeftIcon, IconButton } from "evergreen-ui";
import { observer } from "mobx-react-lite";
import React from "react";
import { useNavigate } from "react-router-dom";
import Titlebar from "../../titlebar/macos";
import { ErrorContent } from "./EditorErrorBoundary";
import { Separator } from "./editor/components/Separator";

export interface LoadingComponentProps {
error?: Error | null;
documentId?: string;
journal?: string;
}

export const placeholderDate = new Date().toISOString().slice(0, 10);
Expand All @@ -30,13 +33,13 @@ export const EditLoadingComponent = observer((props: LoadingComponentProps) => {
</IconButton>
<Separator orientation="vertical" />
</Titlebar>
<Pane padding={50} paddingTop={98} flexGrow={1} display="flex">
<Pane flexGrow={1} display="flex" flexDirection="column" width="100%">
<Pane flexGrow={1} paddingTop={24}>
{props.error && props.error?.message}
</Pane>
</Pane>
</Pane>
{props.error && (
<ErrorContent
error={props.error}
journal={props.journal}
documentId={props.documentId}
/>
)}
</>
);
});
Loading

0 comments on commit 1105ff5

Please sign in to comment.