Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

improve document error handling #310

Merged
merged 1 commit into from
Feb 3, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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