use effect rpc for downloading videos #1064
Conversation
WalkthroughIntroduces a new video download flow using an Effect-based RPC: the frontend requests download info via VideoGetDownloadInfo, fetches the file via HttpClient, and triggers a browser download. Backend adds Videos.getDownloadInfo, wires the new RPC, and the runtime includes FetchHttpClient. Domain schema includes the new RPC. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant UI as CapCard (Frontend)
participant RPC as VideoRpcs (Backend)
participant SVC as Videos Service
participant S3 as S3 Bucket
Note over UI: User clicks "Download"
UI->>RPC: VideoGetDownloadInfo(videoId)
RPC->>SVC: getDownloadInfo(videoId)
SVC->>S3: getSignedObjectUrl(MP4 key)
S3-->>SVC: Signed URL
SVC-->>RPC: Option({ fileName, downloadUrl })
RPC-->>UI: Option({ fileName, downloadUrl })
alt Some(download info)
UI->>UI: HttpClient GET(downloadUrl)
UI->>UI: ArrayBuffer -> Blob -> blob URL
UI->>UI: Trigger browser download (fileName)
else None
UI->>UI: Throw/handle error
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~30 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
packages/web-domain/src/Video.ts (1)
159-170: Tighten the success schema (non-empty filename and URL validation).The RPC looks good. Consider strengthening the schema to avoid empty strings and catch malformed URLs at the boundary.
Apply this diff to make filename non-empty (and optionally refine the URL if you have a shared Url schema/brand):
- success: Schema.Option( - Schema.Struct({ fileName: Schema.String, downloadUrl: Schema.String }), - ), + success: Schema.Option( + Schema.Struct({ + fileName: Schema.NonEmptyString, + downloadUrl: Schema.String, + }), + ),If you maintain a Url schema/brand in the codebase, prefer using it for downloadUrl instead of a plain string.
packages/web-backend/src/Videos/index.ts (1)
135-161: Consider setting filename via signed URL and sanitizing the returned fileName.The logic is correct. Two enhancements to improve UX and robustness:
- Ask S3/CloudFront to set Content-Disposition with a sanitized filename in the signed URL so direct-navigation downloads get a friendly name without needing client blob wrapping.
- Sanitize video.name to remove/replace invalid filename characters before appending “.mp4”.
Example adjustment within the mapped object (pseudocode; depends on your signing helper capabilities):
// When generating the signed URL: bucket.getSignedObjectUrl(v.getFileKey(), { responseContentDisposition: `attachment; filename="${sanitizeFileName(video.name)}.mp4"`, })If you prefer keeping the current client-driven filename approach, at least sanitize before returning:
fileName: `${sanitizeFileName(video.name)}.mp4`I can provide a small sanitizeFileName helper if you want it added here.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx(3 hunks)apps/web/lib/EffectRuntime.ts(2 hunks)packages/web-backend/src/Videos/VideosRpcs.ts(1 hunks)packages/web-backend/src/Videos/index.ts(2 hunks)packages/web-domain/src/Video.ts(1 hunks)
🧰 Additional context used
📓 Path-based instructions (6)
**/*.{ts,tsx,js,jsx,rs}
📄 CodeRabbit inference engine (CLAUDE.md)
Do not add inline, block, or docstring comments in any language; code must be self-explanatory
Files:
packages/web-backend/src/Videos/index.tspackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/lib/EffectRuntime.tspackages/web-domain/src/Video.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Use strict TypeScript and avoid any; leverage shared types from packages
**/*.{ts,tsx}: Use a 2-space indent for TypeScript code.
Use Biome for formatting and linting TypeScript/JavaScript files by runningpnpm format.
Files:
packages/web-backend/src/Videos/index.tspackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/lib/EffectRuntime.tspackages/web-domain/src/Video.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
**/*.{ts,tsx,js,jsx}: Use kebab-case for filenames for TypeScript/JavaScript modules (e.g.,user-menu.tsx).
Use PascalCase for React/Solid components.
Files:
packages/web-backend/src/Videos/index.tspackages/web-backend/src/Videos/VideosRpcs.tsapps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/lib/EffectRuntime.tspackages/web-domain/src/Video.ts
apps/web/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
apps/web/**/*.{ts,tsx}: Use TanStack Query v5 for all client-side server state and data fetching in the web app
Web mutations should call Server Actions directly and perform targeted cache updates with setQueryData/setQueriesData rather than broad invalidations
Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/lib/EffectRuntime.ts
apps/web/app/**/*.{tsx,ts}
📄 CodeRabbit inference engine (CLAUDE.md)
Prefer Server Components for initial data in the Next.js App Router and pass initialData to client components
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx
apps/web/**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
On the client, always use
useEffectQueryoruseEffectMutationfrom@/lib/EffectRuntime; never callEffectRuntime.run*directly in components.
Files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/lib/EffectRuntime.ts
🧠 Learnings (2)
📚 Learning: 2025-09-22T14:19:55.985Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: AGENTS.md:0-0
Timestamp: 2025-09-22T14:19:55.985Z
Learning: Applies to apps/web/**/*.{ts,tsx,js,jsx} : On the client, always use `useEffectQuery` or `useEffectMutation` from `@/lib/EffectRuntime`; never call `EffectRuntime.run*` directly in components.
Applied to files:
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsxapps/web/lib/EffectRuntime.ts
📚 Learning: 2025-09-22T14:17:47.380Z
Learnt from: CR
PR: CapSoftware/Cap#0
File: CLAUDE.md:0-0
Timestamp: 2025-09-22T14:17:47.380Z
Learning: Applies to apps/web/**/*.{ts,tsx} : Client code should use useEffectQuery/useEffectMutation and useRpcClient from apps/web/lib/EffectRuntime.ts; do not create ManagedRuntime inside components
Applied to files:
apps/web/lib/EffectRuntime.ts
🧬 Code graph analysis (5)
packages/web-backend/src/Videos/index.ts (2)
packages/web-domain/src/Video.ts (3)
Video(14-59)VideoId(10-10)VideoId(11-11)packages/web-backend/src/S3Buckets/index.ts (1)
S3Buckets(14-196)
packages/web-backend/src/Videos/VideosRpcs.ts (2)
packages/web-backend/src/Auth.ts (1)
provideOptionalAuth(87-109)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)
apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx (3)
apps/web/lib/EffectRuntime.ts (1)
useEffectMutation(24-24)apps/web/lib/Rpcs.ts (1)
withRpc(16-17)packages/web-backend/src/Workflows.ts (1)
HttpClient(23-26)
apps/web/lib/EffectRuntime.ts (2)
apps/web/lib/Rpcs.ts (1)
Rpc(11-14)apps/web/lib/server.ts (1)
TracingLayer(27-27)
packages/web-domain/src/Video.ts (4)
apps/web/lib/Rpcs.ts (1)
Rpc(11-14)packages/web-domain/src/Folder.ts (1)
NotFoundError(14-17)packages/web-domain/src/Errors.ts (1)
InternalError(3-6)packages/web-domain/src/Policy.ts (1)
PolicyDeniedError(19-22)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (4)
- GitHub Check: Build Desktop (aarch64-apple-darwin, macos-latest)
- GitHub Check: Build Desktop (x86_64-pc-windows-msvc, windows-latest)
- GitHub Check: Vercel Agent Review
- GitHub Check: Analyze (rust)
🔇 Additional comments (3)
apps/web/lib/EffectRuntime.ts (1)
13-17: LGTM: HttpClient layer added to the runtime.Including FetchHttpClient.layer in the merged runtime is correct and aligns with the new client-side download flow.
packages/web-backend/src/Videos/VideosRpcs.ts (1)
34-42: LGTM: New RPC handler with appropriate error mapping.provideOptionalAuth and mapping DatabaseError/UnknownException/S3Error to InternalError are consistent with existing patterns. Propagating policy/password errors is correct.
packages/web-backend/src/Videos/index.ts (1)
18-25: LGTM: Local getById helper with policy + tracing.Factoring getById with Policy.withPublicPolicy and Effect.withSpan improves cohesion and reuse.
| const downloadMutation = useEffectMutation({ | ||
| mutationFn: () => | ||
| Effect.gen(function* () { | ||
| const result = yield* withRpc((r) => r.VideoGetDownloadInfo(cap.id)); | ||
| const httpClient = yield* HttpClient.HttpClient; | ||
| if (Option.isSome(result)) { | ||
| const fetchResponse = yield* httpClient.get(result.value.downloadUrl); | ||
| const blob = yield* fetchResponse.arrayBuffer; | ||
|
|
||
| const blobUrl = window.URL.createObjectURL(new Blob([blob])); | ||
| const link = document.createElement("a"); | ||
| link.href = blobUrl; | ||
| link.download = result.value.fileName; | ||
| link.style.display = "none"; | ||
| document.body.appendChild(link); | ||
| link.click(); | ||
| document.body.removeChild(link); | ||
|
|
||
| window.URL.revokeObjectURL(blobUrl); | ||
| } else { | ||
| throw new Error("Failed to get download URL"); | ||
| } | ||
| }), | ||
| }); |
There was a problem hiding this comment.
Fix: call arrayBuffer() and guard on non-2xx responses before creating the Blob.
Currently arrayBuffer is referenced but not invoked, and the response status isn’t checked. This can break downloads or save error pages.
Apply this diff:
const downloadMutation = useEffectMutation({
mutationFn: () =>
Effect.gen(function* () {
const result = yield* withRpc((r) => r.VideoGetDownloadInfo(cap.id));
const httpClient = yield* HttpClient.HttpClient;
if (Option.isSome(result)) {
- const fetchResponse = yield* httpClient.get(result.value.downloadUrl);
- const blob = yield* fetchResponse.arrayBuffer;
-
- const blobUrl = window.URL.createObjectURL(new Blob([blob]));
+ const res = yield* httpClient.get(result.value.downloadUrl);
+ if (res.status < 200 || res.status >= 300) {
+ throw new Error(`Download failed (${res.status})`);
+ }
+ const bytes = yield* res.arrayBuffer();
+ const blobUrl = window.URL.createObjectURL(
+ new Blob([bytes], { type: "video/mp4" }),
+ );
const link = document.createElement("a");
link.href = blobUrl;
link.download = result.value.fileName;
link.style.display = "none";
document.body.appendChild(link);
link.click();
document.body.removeChild(link);
window.URL.revokeObjectURL(blobUrl);
} else {
throw new Error("Failed to get download URL");
}
}),
});Optional follow-ups:
- Delay revokeObjectURL slightly (setTimeout) to avoid edge-case races in some browsers.
- Consider streaming strategies for very large files to reduce memory pressure (if/when feasible in your target browsers).
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const downloadMutation = useEffectMutation({ | |
| mutationFn: () => | |
| Effect.gen(function* () { | |
| const result = yield* withRpc((r) => r.VideoGetDownloadInfo(cap.id)); | |
| const httpClient = yield* HttpClient.HttpClient; | |
| if (Option.isSome(result)) { | |
| const fetchResponse = yield* httpClient.get(result.value.downloadUrl); | |
| const blob = yield* fetchResponse.arrayBuffer; | |
| const blobUrl = window.URL.createObjectURL(new Blob([blob])); | |
| const link = document.createElement("a"); | |
| link.href = blobUrl; | |
| link.download = result.value.fileName; | |
| link.style.display = "none"; | |
| document.body.appendChild(link); | |
| link.click(); | |
| document.body.removeChild(link); | |
| window.URL.revokeObjectURL(blobUrl); | |
| } else { | |
| throw new Error("Failed to get download URL"); | |
| } | |
| }), | |
| }); | |
| const downloadMutation = useEffectMutation({ | |
| mutationFn: () => | |
| Effect.gen(function* () { | |
| const result = yield* withRpc((r) => r.VideoGetDownloadInfo(cap.id)); | |
| const httpClient = yield* HttpClient.HttpClient; | |
| if (Option.isSome(result)) { | |
| const res = yield* httpClient.get(result.value.downloadUrl); | |
| if (res.status < 200 || res.status >= 300) { | |
| throw new Error(`Download failed (${res.status})`); | |
| } | |
| const bytes = yield* res.arrayBuffer(); | |
| const blobUrl = window.URL.createObjectURL( | |
| new Blob([bytes], { type: "video/mp4" }), | |
| ); | |
| const link = document.createElement("a"); | |
| link.href = blobUrl; | |
| link.download = result.value.fileName; | |
| link.style.display = "none"; | |
| document.body.appendChild(link); | |
| link.click(); | |
| document.body.removeChild(link); | |
| window.URL.revokeObjectURL(blobUrl); | |
| } else { | |
| throw new Error("Failed to get download URL"); | |
| } | |
| }), | |
| }); |
🤖 Prompt for AI Agents
In apps/web/app/(org)/dashboard/caps/components/CapCard/CapCard.tsx around lines
116-139, the download flow incorrectly references fetchResponse.arrayBuffer
without invoking it and doesn’t guard on non-2xx responses; update the code to
check fetchResponse.ok (or status) and throw a descriptive error if not OK, then
call and await the arrayBuffer() method (i.e., invoke arrayBuffer()) before
creating the Blob and proceeding with the link download; optionally delay
URL.revokeObjectURL with setTimeout to avoid race conditions.
Summary by CodeRabbit
New Features
Bug Fixes