Introduce makeLoader to eliminate manual Effect error handling from route loaders
agent:in-progresssource:architecture-review
## Architecture review
### Files
**Existing extraction (actions only):**
- `app/services/route-action.server.ts` (92 LOC) — `makeAction` wraps 67 action routes with declarative error handling
**Route loaders with manual error-handling epilogue (37 files):**
- `app/routes/_app._index.tsx`
- `app/routes/_app.archived-courses.tsx`
- `app/routes/_app.courses.$courseId._index.tsx`
- `app/routes/_app.courses.$courseId.changelog.tsx`
- `app/routes/_app.courses.$courseId.publish.tsx`
- `app/routes/_app.courses.$courseId.versions.$versionId.media-files.tsx`
- `app/routes/_app.pitches.$pitchId.tsx`
- `app/routes/_app.pitches._index.tsx`
- `app/routes/_app.tsx`
- `app/routes/_app.videos.$videoId.ai-hero.tsx`
- `app/routes/_app.videos.$videoId.edit.tsx`
- `app/routes/_app.videos.$videoId.move-to-course.tsx`
- `app/routes/_app.videos.$videoId.newsletter.tsx`
- `app/routes/_app.videos.$videoId.post.tsx`
- `app/routes/_app.videos.$videoId.skills-changelog.tsx`
- `app/routes/_app.videos.$videoId.social.tsx`
- `app/routes/_app.videos.$videoId.thumbnails.tsx`
- `app/routes/_app.videos.$videoId.tsx`
- `app/routes/_app.videos.$videoId.write.tsx`
- `app/routes/_app.videos._index.tsx`
- `app/routes/_app.videos.concatenate.tsx`
- `app/routes/api.auth.ai-hero.status.ts`
- `app/routes/api.auth.google.callback.ts`
- `app/routes/api.auth.google.initiate.ts`
- `app/routes/api.diagram-snapshots.$snapshotId.ts`
- `app/routes/api.diagrams.$diagramId.head.ts`
- `app/routes/api.diagrams.$diagramId.snapshots.list.ts`
- `app/routes/api.diagrams.list.ts`
- `app/routes/api.lesson-files.read.ts`
- `app/routes/api.links.ts`
- `app/routes/api.standalone-files.read.ts`
- `app/routes/api.thumbnails.$thumbnailId.image.ts`
- `app/routes/api.thumbnails.$thumbnailId.layer.$layerType.ts`
- `app/routes/api.videos.$videoId.suggest-chapters.ts`
- `app/routes/clips.$clipId.first-frame.ts`
- `app/routes/clips.$clipId.last-frame.ts`
- `app/routes/diagram-playground._index.tsx`
**Non-makeAction action routes with manual error handling (5 files):**
- `app/routes/clips.transcribe.ts`
- `app/routes/videos.$videoId.completions.ts`
- `app/routes/videos.$videoId.document-completions.ts`
- `app/routes/videos.$videoId.export-to-davinci-resolve.ts`
- `app/routes/videos.$videoId.suggest-next-clip.ts`
### Problem
`makeAction` already provides a **deep module** for action routes: a small declarative interface (an error-tag-to-status map + an Effect program) hides the full error-handling protocol (console logging, tag-based HTTP status mapping, fallback 500, runtime execution). 67 action routes use it. But route loaders — which handle the exact same protocol — have no equivalent. Each of the 37 loader routes independently repeats the same error-handling epilogue:
```typescript
}).pipe(
Effect.tapErrorCause((e) => Console.dir(e, { depth: null })),
Effect.catchTag("NotFoundError", () => {
return Effect.die(data("Video not found", { status: 404 }));
}),
Effect.catchAll(() => {
return Effect.die(data("Internal server error", { status: 500 }));
}),
runtimeLive.runPromise
);
```
This 6–10 line block is copy-pasted across every loader, with minor variations:
- **Which error tags to catch**: 27 loaders catch `NotFoundError`, 7 catch `ParseError`, 4 catch `ConfigError`, and 6 catch domain-specific tags (`AiHeroAuthError`, `GoogleOAuthError`, etc.). 10 loaders catch only `catchAll` with no specific tags.
- **Ad-hoc error messages**: "Video not found", "File not found", "Not found", "Not Found", "Clip not found", "Thumbnail not found", "Pitch not found" — seven different phrasings for the same 404 status. `makeAction`'s `statusMessage` function already standardizes this to "Not found" for 404.
- **Inconsistent logging**: 2 routes use `Console.log(e)` instead of `Console.dir(e, { depth: null })`.
The 37 loader modules are **shallow** at the error-handling seam: their **interface** (which error tags to catch, which status codes to return) is the same complexity as their **implementation** (the pipe chain that maps tags to HTTP responses). The implementation is ~8 LOC of pure protocol mechanics repeated 37 times.
**Deletion test:** Imagine deleting `makeLoader`. The error-handling epilogue would reappear across 37 loader routes — each independently constructing the same `tapErrorCause → catchTag → catchAll → runPromise` pipeline. This is exactly the pattern `makeAction` already prevented from reappearing across 67 action routes. `makeLoader` earns its keep: it provides **locality** for the loader error-handling protocol and **leverage** for every route that loads data through Effect.
Five action routes (`clips.transcribe.ts`, `videos.$videoId.completions.ts`, `videos.$videoId.document-completions.ts`, `videos.$videoId.export-to-davinci-resolve.ts`, `videos.$videoId.suggest-next-clip.ts`) also use the manual pattern instead of `makeAction` — likely because they pre-date `makeAction`'s introduction or have non-standard input handling. These can be migrated to `makeAction` as a follow-on.
Consequences of the current gap:
- **No locality**: the error-tag → HTTP-status mapping is scattered across 37 files. `makeAction` centralizes this for actions; loaders have no equivalent. Changing the convention (e.g., logging to a structured logger instead of `Console.dir`) requires editing 37 files.
- **No leverage**: every new page route that loads data through Effect must copy the epilogue from an existing route. The 37th loader paid the same boilerplate cost as the 2nd.
- **Behavioral divergence**: error messages have already diverged — 7 different 404 phrasings exist across loaders. Two routes use `Console.log` instead of `Console.dir`. These are not intentional differences; they're copy-paste drift.
- **Asymmetric codebase**: actions have a deep, declarative error-handling module (`makeAction`). Loaders have shallow, imperative error handling. A developer adding a new route must learn two patterns depending on whether they're writing a loader or an action.
### Solution
Introduce `makeLoader` in `app/services/route-action.server.ts` (alongside `makeAction`) that wraps an Effect-based loader with the same declarative error-handling protocol. Each loader replaces its manual epilogue with a single `makeLoader({ errors, effect })` call.
### Benefits
- **Locality**: all knowledge about how Effect errors map to HTTP responses in loaders lives in `makeLoader` — same as `makeAction` does for actions. Changing the logging strategy, adding a structured error format, or integrating tracing is a single edit.
- **Leverage**: the `makeLoader` interface is small (an error map + an Effect program parameterized by route params) but its implementation handles: console error logging, tag-based status mapping with standardized messages, catchAll fallback to 500, and runtime execution. Adding a new loader drops from ~8 LOC of boilerplate to zero.
- **Test surface improvement**: the error-handling protocol is tested once in `makeLoader`'s test suite (correct status for each tag, fallback for unknown tags, error logging). Currently this behavior is implicitly "tested" by being copy-pasted — no route actually tests its error handling.
- **Consistency by construction**: `makeLoader` uses the same `statusMessage` function as `makeAction`, so "Not found" replaces the 7 variant phrasings. The `tapErrorCause` logging is standardized. No drift possible.
### Before / After diagram
```mermaid
graph TD
subgraph "Before — asymmetric error handling"
MA["makeAction<br/>92 LOC<br/>Declarative error map"]
A1["api.courses.add.ts"] --> MA
A2["api.videos.delete.ts"] --> MA
A3["...65 more action routes"] --> MA
L1["_app._index.tsx<br/>Manual pipe epilogue"] --- E1["Own tapErrorCause +<br/>catchAll"]
L2["_app.videos.$videoId.tsx<br/>Manual pipe epilogue"] --- E2["Own tapErrorCause +<br/>catchTag + catchAll"]
L3["_app.pitches.$pitchId.tsx<br/>Manual pipe epilogue"] --- E3["Own tapErrorCause +<br/>catchTag + catchAll"]
L4["...34 more loader routes<br/>Manual pipe epilogue"] --- E4["Own tapErrorCause +<br/>variant error messages"]
end
```
```mermaid
graph TD
subgraph "After — symmetric declarative error handling"
MA["makeAction<br/>92 LOC<br/>Declarative error map"]
ML["makeLoader<br/>~45 LOC<br/>Declarative error map"]
SH["statusMessage + error pipeline<br/>Shared between makeAction and makeLoader"]
MA --> SH
ML --> SH
A1["api.courses.add.ts"] --> MA
A2["api.videos.delete.ts"] --> MA
A3["...65 more action routes"] --> MA
L1["_app._index.tsx"] --> ML
L2["_app.videos.$videoId.tsx"] --> ML
L3["_app.pitches.$pitchId.tsx"] --> ML
L4["...34 more loader routes"] --> ML
end
```
### Recommendation strength
Strong
---
## Problem Statement
The codebase has a proven deep module for action-route error handling (`makeAction`, 92 LOC, used by 67 action routes) but no equivalent for loaders. 37 loader routes each manually repeat a 6–10 line Effect error-handling epilogue: `tapErrorCause` for logging, zero or more `catchTag` calls mapping domain errors to HTTP statuses, a `catchAll` fallback to 500, and `runtimeLive.runPromise`. The epilogue is structurally identical across all 37 loaders — the only variation is which error tags each loader catches and what ad-hoc message string it uses (7 different phrasings for 404 alone). This duplication has already caused minor behavioral divergence: inconsistent error messages ("Video not found" vs "Not Found" vs "File not found"), inconsistent logging calls (`Console.dir` vs `Console.log`), and an asymmetric developer experience where actions have declarative error handling but loaders require imperative boilerplate.
## Solution
Add a `makeLoader` function to `app/services/route-action.server.ts` alongside the existing `makeAction`:
```typescript
interface MakeLoaderConfig<A, E, R> {
errors?: { [K in ErrorTags<E>]?: number };
effect: (ctx: {
params: Record<string, string | undefined>;
}) => Effect.Effect<A, E, R>;
}
export function makeLoader<A, E, R>(
config: MakeLoaderConfig<A, E, R>,
runtime: ManagedRuntime.ManagedRuntime<any, any> = runtimeLive
): (args: {
params: Record<string, string | undefined>;
}) => Promise<A> {
const errorMap: Record<string, number> = {
ParseError: 400,
NotFoundError: 404,
...config.errors,
};
return (args) => {
const pipeline = config.effect({ params: args.params }).pipe(
Effect.tapErrorCause((e) => Console.dir(e, { depth: null })),
Effect.catchAll((error: NoInfer<E>) => {
const tag =
error != null &&
typeof error === "object" &&
"_tag" in error &&
typeof (error as Record<string, unknown>)._tag === "string"
? ((error as Record<string, unknown>)._tag as string)
: undefined;
const status =
tag !== undefined && tag in errorMap ? errorMap[tag]! : 500;
return Effect.die(data(statusMessage(status), { status }));
})
);
return runtime.runPromise(pipeline as Effect.Effect<A, never, R>);
};
}
```
Key differences from `makeAction`:
- No `input` parameter (loaders don't parse request bodies)
- No `dump` parameter (database dumps are an action concern)
- `NotFoundError -> 404` is a default mapping (the most common loader tag, used by 27/37 loaders)
- The `ctx` object exposes only `params` (no `payload`)
The shared `statusMessage` function and error-pipeline logic can be factored into a private helper used by both `makeAction` and `makeLoader`, eliminating duplication between the two.
### Migration per loader
**Before** (`_app.videos.$videoId.post.tsx`):
```typescript
export const loader = async (args: Route.LoaderArgs) => {
return Effect.gen(function* () {
const ctx = yield* loadVideoPostingContext(args.params.videoId!);
// ... destination-specific data
return { ...ctx, isYoutubeAuthenticated: youtubeAuth !== null };
}).pipe(
Effect.tapErrorCause((e) => Console.dir(e, { depth: null })),
Effect.catchTag("NotFoundError", () => {
return Effect.die(data("Video not found", { status: 404 }));
}),
Effect.catchAll(() => {
return Effect.die(data("Internal server error", { status: 500 }));
}),
runtimeLive.runPromise
);
};
```
**After**:
```typescript
export const loader = makeLoader({
effect: ({ params }) =>
Effect.gen(function* () {
const ctx = yield* loadVideoPostingContext(params.videoId!);
// ... destination-specific data
return { ...ctx, isYoutubeAuthenticated: youtubeAuth !== null };
}),
});
```
The 8 lines of error-handling epilogue disappear. The `NotFoundError -> 404` mapping is a default; only loaders with non-standard tags need to declare `errors`.
**Before** (`_app._index.tsx`, catchAll-only):
```typescript
export const loader = async (args: Route.LoaderArgs) => {
return Effect.gen(function* () {
// ... data assembly
}).pipe(
Effect.tapErrorCause((e) => Console.dir(e, { depth: null })),
Effect.catchAll(() => {
return Effect.die(data("Internal server error", { status: 500 }));
}),
runtimeLive.runPromise
);
};
```
**After**:
```typescript
export const loader = makeLoader({
effect: ({ params }) =>
Effect.gen(function* () {
// ... data assembly
}),
});
```
## User Stories
- As a developer adding a new page route with a loader, I write `makeLoader({ effect })` and get standard error handling (logging, tag-based HTTP statuses, 500 fallback) automatically. Today I copy ~8 lines of error-handling boilerplate from an existing route and risk introducing variant error messages.
- As a developer changing the error-logging strategy (e.g., switching from `Console.dir` to a structured logger), I change `makeLoader` once and all 37 loaders inherit the update. Today I edit 37 files and risk missing the 2 that use `Console.log` instead of `Console.dir`.
- As a developer reviewing a loader PR, I see only the data-loading logic — no error-handling boilerplate to verify matches the convention. The error handling is declarative: either a custom `errors` map or the defaults.
- As a developer looking at a route file for the first time, I see the same `makeLoader` / `makeAction` pattern regardless of whether it's a loader or action. Today, actions use `makeAction` and loaders use a manual pipe — two patterns for the same concern.
- As a developer writing tests for error handling, I test `makeLoader` once (correct status for `NotFoundError`, `ParseError`, unknown tags, custom-mapped tags) instead of relying on 37 routes to each implement it correctly.
## Implementation Decisions
1. **`makeLoader` lives alongside `makeAction` in `route-action.server.ts`.** Both functions implement the same concern (Effect error -> HTTP response) for different route entry points. They share `statusMessage` and the error-mapping pipeline. A new module name like `route-handlers.server.ts` would be cleaner but renaming the existing module is churn — the file already hosts `makeAction` and is the natural home.
2. **`NotFoundError -> 404` is a built-in default.** 27 of 37 loaders catch `NotFoundError`. Making it a default means the majority of loaders need no `errors` map at all. `makeAction` defaults only `ParseError -> 400`; `makeLoader` adds `NotFoundError -> 404` because loaders overwhelmingly fetch-by-ID.
3. **No `request` in the loader context.** Loaders don't parse request bodies. The `effect` callback receives only `{ params }`. If a loader needs the raw request (e.g., for auth headers), it can close over `args.request` — but this is rare enough not to warrant a first-class parameter.
4. **No `dump` parameter.** `makeAction`'s `dump` option wraps the effect with `withDatabaseDump` for mutation safety. Loaders are read-only; database dumps don't apply.
5. **Extract shared error pipeline.** The tag-inspection, status-mapping, and `Effect.die(data(...))` logic is identical between `makeAction` and `makeLoader`. Extract it into a private `buildErrorPipeline(errorMap)` function to avoid duplicating the error-handling implementation within `route-action.server.ts`.
6. **Incremental migration.** Introduce `makeLoader` first, then migrate loaders one at a time. Each migration is independently deployable and produces identical HTTP behavior (modulo standardizing variant error messages to the canonical form). Start with the simplest loaders (catchAll-only, e.g., `_app._index.tsx`, `_app.archived-courses.tsx`) to validate the pattern.
7. **Migrate straggler actions as follow-on.** The 5 action routes still using manual error handling (`clips.transcribe.ts`, `videos.$videoId.completions.ts`, etc.) can be migrated to `makeAction` in the same sweep, but this is not required by this PRD.
## Testing Decisions
- `makeLoader` gets a focused test suite verifying: `NotFoundError` produces 404, `ParseError` produces 400, custom-mapped tags produce their declared status, unmapped tags produce 500, error cause is logged via `Console.dir`, and the effect's successful return value is passed through unchanged.
- These tests run against a test `ManagedRuntime` with a minimal Layer — no database, no filesystem. The error-handling protocol is pure logic.
- The shared `buildErrorPipeline` helper (if extracted) is tested once and shared between `makeAction` and `makeLoader` test suites.
- Migrated loader routes do not need new tests for error handling — that's covered by `makeLoader`'s test suite. They retain any existing tests for their data-loading logic.
- A snapshot-style integration test can verify 2–3 representative loaders (one catchAll-only, one with `NotFoundError`, one with a custom tag) to confirm the migration produces identical HTTP responses.
## Out of Scope
- Refactoring `makeAction` itself. Its implementation is stable and used by 67 routes. The shared error pipeline extraction is the only change to existing `makeAction` code.
- Migrating the 5 straggler action routes to `makeAction`. They can adopt `makeAction` independently as a follow-on.
- Changing the error types or introducing new error tags. This PRD extracts the existing error-handling convention; new error types are additive.
- Adding structured logging, tracing, or metrics. `makeLoader` makes these enhancements trivial (single edit), but adding them is follow-on work.
- Changing the `runtimeLive` Layer or Effect service composition. The runtime is passed through unchanged.
- Modifying the `data()` helper from React Router or the HTTP status codes used.
## Further Notes
- 37 loaders x ~8 LOC of epilogue = ~296 LOC of duplicated error handling. After migration, this moves to ~45 LOC in `makeLoader` + ~15 LOC of shared pipeline extraction. Net reduction: ~235 LOC and elimination of per-loader error-handling boilerplate.
- The 5 straggler action routes add ~50 LOC of duplication that `makeAction` already solved. Migrating them is trivial follow-on work.
- The codebase currently has 47 loaders total. 37 use the manual pattern; the remaining 10 either don't use Effect or have non-standard requirements. After migration, ~80% of all loaders use `makeLoader`.
- Error message standardization: the 7 variant 404 phrasings ("Video not found", "File not found", "Not found", "Not Found", "Clip not found", "Thumbnail not found", "Pitch not found") consolidate to `makeLoader`'s standard "Not found". This is intentional — the HTTP status code communicates the error class; the message need not name the resource type.
- The asymmetry between `makeAction` (67 routes, declarative) and manual loader error handling (37 routes, imperative) is the strongest signal that this extraction is overdue. The same team that built `makeAction` would recognize `makeLoader` as the natural completion.
- `makeLoader` also enables future enhancements at the loader infrastructure level: request-scoped logging context, OpenTelemetry span creation, cache-control header injection. These are all single-edit additions once the extraction exists.
0 条评论