Skip to content
Open
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
6 changes: 6 additions & 0 deletions .server-changes/env-not-found-404.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
---
area: webapp
type: fix
---

Dashboard runs, sessions, batches, and schedule-detail loaders now return 404 (or redirect to the user's home with a toast for missing projects) instead of 500 when a slug doesn't resolve.
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ import {
v3BatchPath,
v3BatchRunsPath,
} from "~/utils/pathBuilder";
import { throwNotFound } from "~/utils/httpErrors";

export const meta: MetaFunction = () => {
return [
Expand All @@ -74,7 +75,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {

const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
throw new Error("Environment not found");
throwNotFound("Environment not found");
}

const url = new URL(request.url);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ import { useOrganization } from "~/hooks/useOrganizations";
import { useProject } from "~/hooks/useProject";
import { useSearchParams } from "~/hooks/useSearchParam";
import { useShortcutKeys } from "~/hooks/useShortcutKeys";
import { redirectWithErrorMessage } from "~/models/message.server";
import { findProjectBySlug } from "~/models/project.server";
import { findEnvironmentBySlug } from "~/models/runtimeEnvironment.server";
import { getRunFiltersFromRequest } from "~/presenters/RunFilters.server";
Expand All @@ -59,6 +60,7 @@ import {
v3TestPath,
v3TestTaskPath,
} from "~/utils/pathBuilder";
import { throwNotFound } from "~/utils/httpErrors";
import { ListPagination } from "../../components/ListPagination";
import { CreateBulkActionInspector } from "../resources.orgs.$organizationSlug.projects.$projectParam.env.$envParam.runs.bulkaction";
import { Callout } from "~/components/primitives/Callout";
Expand All @@ -77,12 +79,12 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {

const project = await findProjectBySlug(organizationSlug, projectParam, userId);
if (!project) {
throw new Error("Project not found");
return redirectWithErrorMessage("/", request, "Project not found");
}

const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
throw new Error("Environment not found");
throwNotFound("Environment not found");
}

const filters = await getRunFiltersFromRequest(request);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ import {
v3SchedulePath,
v3SchedulesPath,
} from "~/utils/pathBuilder";
import { throwNotFound } from "~/utils/httpErrors";
import { DeleteTaskScheduleService } from "~/v3/services/deleteTaskSchedule.server";
import { SetActiveOnTaskScheduleService } from "~/v3/services/setActiveOnTaskSchedule.server";

Expand Down Expand Up @@ -84,7 +85,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {
});

if (!result) {
throw new Error("Schedule not found");
throwNotFound("Schedule not found");
}

return typedjson({ schedule: result.schedule });
Expand Down
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🚩 Sessions detail route has two different 404 throw styles in the same loader

In sessions.$sessionParam/route.tsx, the environment-not-found check (line 75) now uses throwNotFound("Environment not found") which throws new Response(undefined, { status: 404, statusText }), while the session-not-found check (line 88) uses the pre-existing throw new Response("Session not found", { status: 404 }) which puts the message in the body instead of statusText. Both produce 404s, but the error boundary at _app/route.tsx:34-43 renders them differently: the throwNotFound style passes the message via statusText to friendlyErrorDisplay, while the body-based style results in error.data being the string "Session not found" and error.data.message being undefined, falling through to the generic friendlyErrorDisplay(404, "").message which returns empty string (since "" ?? fallback doesn't trigger). Consider standardizing on throwNotFound for both for consistent user-facing messages.

(Refers to lines 87-88)

Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,7 @@ import {
v3RunsPath,
v3SessionsPath,
} from "~/utils/pathBuilder";
import { throwNotFound } from "~/utils/httpErrors";

const ParamsSchema = EnvironmentParamSchema.extend({
sessionParam: z.string(),
Expand All @@ -71,7 +72,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {

const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
throw new Error("Environment not found");
throwNotFound("Environment not found");
}

const presenter = new SessionPresenter($replica);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import { SessionListPresenter } from "~/presenters/v3/SessionListPresenter.serve
import { clickhouseClient } from "~/services/clickhouseInstance.server";
import { requireUserId } from "~/services/session.server";
import { docsPath, EnvironmentParamSchema } from "~/utils/pathBuilder";
import { throwNotFound } from "~/utils/httpErrors";

export const meta: MetaFunction = () => {
return [
Expand All @@ -39,7 +40,7 @@ export const loader = async ({ request, params }: LoaderFunctionArgs) => {

const environment = await findEnvironmentBySlug(project.id, envParam, userId);
if (!environment) {
throw new Error("Environment not found");
throwNotFound("Environment not found");
}

const filters = getSessionFiltersFromRequest(request);
Expand Down
4 changes: 4 additions & 0 deletions apps/webapp/app/utils/httpErrors.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
export function throwNotFound(statusText: string): never {
throw new Response(undefined, { status: 404, statusText });
}
Comment thread
d-cs marked this conversation as resolved.

export function friendlyErrorDisplay(statusCode: number, statusText?: string) {
switch (statusCode) {
case 400:
Expand Down
28 changes: 28 additions & 0 deletions apps/webapp/test/httpErrors.test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
import { describe, expect, it } from "vitest";
import { throwNotFound } from "~/utils/httpErrors";

describe("throwNotFound", () => {
it("throws a Response with status 404 and the provided statusText", () => {
let thrown: unknown;
try {
throwNotFound("Environment not found");
} catch (e) {
thrown = e;
}

expect(thrown).toBeInstanceOf(Response);
expect((thrown as Response).status).toBe(404);
expect((thrown as Response).statusText).toBe("Environment not found");
});

it("passes through whatever statusText the caller provides", () => {
let thrown: unknown;
try {
throwNotFound("Project not found");
} catch (e) {
thrown = e;
}

expect((thrown as Response).statusText).toBe("Project not found");
});
});