Skip to content

Commit

Permalink
fix: show more homepage while workspace files are loading. add timeou…
Browse files Browse the repository at this point in the history
…t and error banner (#2373)
  • Loading branch information
mscolnick committed Sep 20, 2024
1 parent 0099ecd commit 2cd9ec4
Show file tree
Hide file tree
Showing 5 changed files with 147 additions and 75 deletions.
3 changes: 1 addition & 2 deletions frontend/src/components/home/state.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,11 @@ export type RunningNotebooksMap = Map<string, MarimoFile>;
export const RunningNotebooksContext = React.createContext<{
runningNotebooks: RunningNotebooksMap;
setRunningNotebooks: (data: RunningNotebooksMap) => void;
root: string;
}>({
runningNotebooks: new Map(),
root: "",
setRunningNotebooks: Functions.NOOP,
});
export const WorkspaceRootContext = React.createContext<string>("");

export const includeMarkdownAtom = atomWithStorage<boolean>(
"marimo:home:include-markdown",
Expand Down
127 changes: 74 additions & 53 deletions frontend/src/components/pages/home-page.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,7 @@ import {
expandedFoldersAtom,
includeMarkdownAtom,
RunningNotebooksContext,
WorkspaceRootContext,
} from "../home/state";
import { Maps } from "@/utils/maps";
import { Input } from "../ui/input";
Expand All @@ -82,22 +83,19 @@ import {
} from "@/components/ui/dropdown-menu";
import { Objects } from "@/utils/objects";
import { CaretDownIcon } from "@radix-ui/react-icons";
import { ErrorBoundary } from "../editor/boundary/ErrorBoundary";
import { Banner } from "@/plugins/impl/common/error-banner";
import { prettyError } from "@/utils/errors";

function tabTarget(path: string) {
// Consistent tab target so we open in the same tab when clicking on the same notebook
return `${getSessionId()}-${encodeURIComponent(path)}`;
}

const HomePage: React.FC = () => {
const [includeMarkdown, setIncludeMarkdown] = useAtom(includeMarkdownAtom);
const [searchText, setSearchText] = useState("");
const [nonce, setNonce] = useState(0);

const recentsResponse = useAsyncData(() => getRecentFiles(), []);
const workspaceResponse = useAsyncData(
() => getWorkspaceFiles({ includeMarkdown }),
[includeMarkdown],
);

useInterval(
() => {
Expand All @@ -112,11 +110,7 @@ const HomePage: React.FC = () => {
return Maps.keyBy(response.files, (file) => file.path);
}, [nonce]);

const response = combineAsyncData(
recentsResponse,
workspaceResponse,
runningResponse,
);
const response = combineAsyncData(recentsResponse, runningResponse);

if (response.error) {
throw response.error;
Expand All @@ -127,14 +121,13 @@ const HomePage: React.FC = () => {
return <Spinner centered={true} size="xlarge" />;
}

const [recents, workspace, running] = data;
const [recents, running] = data;

return (
<Suspense>
<RunningNotebooksContext.Provider
value={{
runningNotebooks: running,
root: workspace.root,
setRunningNotebooks: runningResponse.setData,
}}
>
Expand All @@ -158,49 +151,77 @@ const HomePage: React.FC = () => {
files={recents.files}
openNewTab={true}
/>
<div className="flex flex-col gap-2">
<Header
Icon={BookTextIcon}
control={
<div className="flex items-center gap-2">
<Input
id="search"
value={searchText}
icon={<SearchIcon size={13} />}
onChange={(e) => setSearchText(e.target.value)}
placeholder="Search"
className="mb-0 border-border"
/>
<CollapseAllButton />
<Checkbox
data-testid="include-markdown-checkbox"
id="include-markdown"
checked={includeMarkdown}
onCheckedChange={(checked) =>
setIncludeMarkdown(Boolean(checked))
}
/>
<Label htmlFor="include-markdown">Include markdown</Label>
</div>
}
>
Workspace
<RefreshCcwIcon
className="w-4 h-4 ml-1 cursor-pointer opacity-70 hover:opacity-100"
onClick={() => workspaceResponse.reload()}
<ErrorBoundary>
<WorkspaceNotebooks />
</ErrorBoundary>
</div>
</RunningNotebooksContext.Provider>
</Suspense>
);
};

const WorkspaceNotebooks: React.FC = () => {
const [includeMarkdown, setIncludeMarkdown] = useAtom(includeMarkdownAtom);
const [searchText, setSearchText] = useState("");
const workspaceResponse = useAsyncData(
() => getWorkspaceFiles({ includeMarkdown }),
[includeMarkdown],
);

if (workspaceResponse.error) {
return (
<Banner kind="danger" className="rounded p-4">
{prettyError(workspaceResponse.error)}
</Banner>
);
}

if (workspaceResponse.loading || !workspaceResponse.data) {
return <Spinner centered={true} size="xlarge" className="mt-6" />;
}

const workspace = workspaceResponse.data;

return (
<WorkspaceRootContext.Provider value={workspace.root}>
<div className="flex flex-col gap-2">
<Header
Icon={BookTextIcon}
control={
<div className="flex items-center gap-2">
<Input
id="search"
value={searchText}
icon={<SearchIcon size={13} />}
onChange={(e) => setSearchText(e.target.value)}
placeholder="Search"
className="mb-0 border-border"
/>
{workspaceResponse.loading && <Spinner size="small" />}
</Header>
<div className="flex flex-col divide-y divide-[var(--slate-3)] border rounded overflow-hidden max-h-[48rem] overflow-y-auto shadow-sm bg-background">
<NotebookFileTree
searchText={searchText}
files={workspace.files}
<CollapseAllButton />
<Checkbox
data-testid="include-markdown-checkbox"
id="include-markdown"
checked={includeMarkdown}
onCheckedChange={(checked) =>
setIncludeMarkdown(Boolean(checked))
}
/>
<Label htmlFor="include-markdown">Include markdown</Label>
</div>
</div>
}
>
Workspace
<RefreshCcwIcon
className="w-4 h-4 ml-1 cursor-pointer opacity-70 hover:opacity-100"
onClick={() => workspaceResponse.reload()}
/>
{workspaceResponse.loading && <Spinner size="small" />}
</Header>
<div className="flex flex-col divide-y divide-[var(--slate-3)] border rounded overflow-hidden max-h-[48rem] overflow-y-auto shadow-sm bg-background">
<NotebookFileTree searchText={searchText} files={workspace.files} />
</div>
</RunningNotebooksContext.Provider>
</Suspense>
</div>
</WorkspaceRootContext.Provider>
);
};

Expand Down Expand Up @@ -285,7 +306,7 @@ const Node = ({ node, style }: NodeRendererProps<FileInfo>) => {

const Icon = FILE_TYPE_ICONS[fileType];
const iconEl = <Icon className="w-5 h-5 flex-shrink-0" strokeWidth={1.5} />;
const root = useContext(RunningNotebooksContext).root;
const root = useContext(WorkspaceRootContext);

const renderItem = () => {
const itemClassName =
Expand Down
7 changes: 7 additions & 0 deletions frontend/src/utils/errors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,13 @@ export function prettyError(error: unknown): string {
}
return maybeExtractDetails(error.message);
}
if (typeof error === "object") {
const details = DetailsSchema.safeParse(error);
if (details.success) {
return details.data.detail;
}
return JSON.stringify(error);
}
try {
return JSON.stringify(error);
} catch {
Expand Down
1 change: 1 addition & 0 deletions marimo/_server/api/status.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ class HTTPStatus(IntEnum):
NOT_MODIFIED = 304
BAD_REQUEST = 400
FORBIDDEN = 403
REQUEST_TIMEOUT = 408
NOT_FOUND = 404
METHOD_NOT_ALLOWED = 405
UNSUPPORTED_MEDIA_TYPE = 415
Expand Down
84 changes: 64 additions & 20 deletions marimo/_server/file_router.py
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,9 @@
import abc
import os
import pathlib
from typing import List, Optional
import signal
from contextlib import contextmanager
from typing import TYPE_CHECKING, Generator, List, Optional

from marimo import _loggers
from marimo._config.config import WidthType
Expand All @@ -15,6 +17,9 @@
from marimo._server.models.home import MarimoFile
from marimo._utils.marimo_path import MarimoPath

if TYPE_CHECKING:
from types import FrameType

LOGGER = _loggers.marimo_logger()

# Some unique identifier for a file
Expand Down Expand Up @@ -209,60 +214,80 @@ def files(self) -> List[FileInfo]:
return self._lazy_files

def _load_files(self) -> List[FileInfo]:
import time

start_time = time.time()
MAX_EXECUTION_TIME = 5 # 5 seconds timeout

def recurse(
directory: str, depth: int = 0
) -> Optional[List[FileInfo]]:
if depth > MAX_DEPTH:
return None

if time.time() - start_time > MAX_EXECUTION_TIME:
raise HTTPException(
status_code=HTTPStatus.REQUEST_TIMEOUT,
detail="Request timed out: Loading workspace files took too long.", # noqa: E501
)

try:
entries = os.listdir(directory)
entries = os.scandir(directory)
except OSError as e:
LOGGER.debug("OSError listing directory: %s", str(e))
LOGGER.debug("OSError scanning directory: %s", str(e))
return None

files: List[FileInfo] = []
folders: List[FileInfo] = []

for entry in entries:
full_path = os.path.join(directory, entry)
if os.path.isdir(full_path):
if entry in skip_dirs or depth == MAX_DEPTH:
# Skip hidden files and directories
if entry.name.startswith("."):
continue

if entry.is_dir():
if entry.name in skip_dirs or depth == MAX_DEPTH:
continue
children = recurse(full_path, depth + 1)
children = recurse(entry.path, depth + 1)
if children:
folders.append(
FileInfo(
id=full_path,
path=full_path,
name=entry,
id=entry.path,
path=entry.path,
name=entry.name,
is_directory=True,
is_marimo_file=False,
children=children,
)
)
else:
if any(
entry.endswith(ext) for ext in allowed_extensions
) and self._is_marimo_app(full_path):
elif entry.name.endswith(tuple(allowed_extensions)):
if self._is_marimo_app(entry.path):
files.append(
FileInfo(
id=full_path,
path=full_path,
name=entry,
id=entry.path,
path=entry.path,
name=entry.name,
is_directory=False,
is_marimo_file=True,
last_modified=os.path.getmtime(full_path),
last_modified=entry.stat().st_mtime,
)
)

# Sort folders then files, based on natural sort (alpha, then num)
return sorted(folders, key=natural_sort_file) + sorted(
files, key=natural_sort_file
)

MAX_DEPTH = 5
skip_dirs = {".git", ".venv", "__pycache__", "node_modules"}
skip_dirs = {
"venv",
"__pycache__",
"node_modules",
"site-packages",
"eggs",
}
allowed_extensions = (
{".py", ".md"} if self.include_markdown else {".py"}
(".py", ".md") if self.include_markdown else (".py",)
)

return recurse(self.directory) or []
Expand All @@ -285,3 +310,22 @@ def get_unique_file_key(self) -> str | None:

def maybe_get_single_file(self) -> MarimoFile | None:
return None


@contextmanager
def timeout(seconds: int, message: str) -> Generator[None, None, None]:
def timeout_handler(signum: int, frame: Optional[FrameType]) -> None:
del signum, frame
raise HTTPException(
status_code=HTTPStatus.REQUEST_TIMEOUT,
detail="Request timed out: {0}".format(message),
)

# Set the timeout handler
original_handler = signal.signal(signal.SIGALRM, timeout_handler)
try:
signal.alarm(seconds)
yield
finally:
signal.alarm(0)
signal.signal(signal.SIGALRM, original_handler)

0 comments on commit 2cd9ec4

Please sign in to comment.