-
Notifications
You must be signed in to change notification settings - Fork 917
use rpc and dataloader for fetching video thumbnails #1170
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,54 @@ | ||
import type { Video } from "@cap/web-domain"; | ||
import { dataLoader } from "@effect/experimental/RequestResolver"; | ||
import { Effect, Request, RequestResolver } from "effect"; | ||
import type { NonEmptyArray } from "effect/Array"; | ||
import { Rpc } from "@/lib/Rpcs"; | ||
|
||
export namespace ThumbnailRequest { | ||
export const queryKey = (videoId: string) => ["thumbnail", videoId] as const; | ||
|
||
export class ThumbnailRequest extends Request.Class< | ||
string, | ||
unknown, | ||
{ videoId: Video.VideoId } | ||
> {} | ||
|
||
export class DataLoaderResolver extends Effect.Service<DataLoaderResolver>()( | ||
"ThumbnailRequest/DataLoaderResolver", | ||
{ | ||
scoped: Effect.gen(function* () { | ||
const rpc = yield* Rpc; | ||
|
||
const requestResolver = RequestResolver.makeBatched( | ||
(requests: NonEmptyArray<ThumbnailRequest>) => { | ||
return rpc.VideosGetThumbnails(requests.map((r) => r.videoId)).pipe( | ||
Effect.flatMap( | ||
// biome-ignore lint/suspicious/useIterableCallbackReturn: effect | ||
Effect.forEach((result, index) => | ||
Effect.matchEffect(Effect.flatten(result), { | ||
onSuccess: (v) => Request.succeed(requests[index]!, v), | ||
onFailure: (e) => Request.fail(requests[index]!, e), | ||
}), | ||
), | ||
), | ||
Effect.catchAll((error) => | ||
Effect.forEach( | ||
requests, | ||
(request) => Request.fail(request, error), | ||
{ | ||
discard: true, | ||
}, | ||
), | ||
), | ||
); | ||
}, | ||
); | ||
|
||
return yield* dataLoader(requestResolver, { | ||
window: "10 millis", | ||
}); | ||
}), | ||
dependencies: [Rpc.Default], | ||
}, | ||
) {} | ||
} |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
@@ -1,5 +1,5 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { InternalError, Video } from "@cap/web-domain"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Effect } from "effect"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Effect, Exit, Schema, Unify } from "effect"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { provideOptionalAuth } from "../Auth.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
import { Videos } from "./index.ts"; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@@ -43,6 +43,33 @@ export const VideosRpcsLive = Video.VideoRpcs.toLayer( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
S3Error: () => new InternalError({ type: "s3" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
VideosGetThumbnails: (videoIds) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.all( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
videoIds.map((id) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
videos.getThumbnailURL(id).pipe( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.catchTags({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DatabaseError: () => new InternalError({ type: "database" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
S3Error: () => new InternalError({ type: "s3" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.matchEffect({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onSuccess: (v) => Effect.succeed(Exit.succeed(v)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
onFailure: (e) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Schema.is(InternalError)(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
? Effect.fail(e) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
: Effect.succeed(Exit.fail(e)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.map((v) => Unify.unify(v)), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
{ concurrency: 10 }, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
).pipe( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
provideOptionalAuth, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.catchTags({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
DatabaseError: () => new InternalError({ type: "database" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
UnknownException: () => new InternalError({ type: "unknown" }), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+47
to
+72
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Per-item errors fail the entire batch instead of being wrapped in Exit. The current error handling converts For a batched endpoint, the typical pattern is:
The frontend resolver in Apply this diff to wrap per-item errors in VideosGetThumbnails: (videoIds) =>
Effect.all(
videoIds.map((id) =>
videos.getThumbnailURL(id).pipe(
- Effect.catchTags({
- DatabaseError: () => new InternalError({ type: "database" }),
- S3Error: () => new InternalError({ type: "s3" }),
- }),
Effect.matchEffect({
onSuccess: (v) => Effect.succeed(Exit.succeed(v)),
- onFailure: (e) =>
- Schema.is(InternalError)(e)
- ? Effect.fail(e)
- : Effect.succeed(Exit.fail(e)),
+ onFailure: (e) => {
+ const mappedError =
+ e._tag === "DatabaseError"
+ ? new InternalError({ type: "database" })
+ : e._tag === "S3Error"
+ ? new InternalError({ type: "s3" })
+ : e;
+ return Effect.succeed(Exit.fail(mappedError));
+ },
}),
- Effect.map((v) => Unify.unify(v)),
),
),
{ concurrency: 10 },
).pipe(
provideOptionalAuth,
Effect.catchTags({
DatabaseError: () => new InternalError({ type: "database" }),
UnknownException: () => new InternalError({ type: "unknown" }),
}),
), This ensures per-item errors are wrapped in 📝 Committable suggestion
Suggested change
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); |
Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -160,6 +160,38 @@ export class Videos extends Effect.Service<Videos>()("Videos", { | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.transposeOption, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
getThumbnailURL: Effect.fn("Videos.getThumbnailURL")(function* ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
videoId: Video.VideoId, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const videoOpt = yield* repo | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.getById(videoId) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
.pipe(Policy.withPublicPolicy(policy.canView(videoId))); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return yield* videoOpt.pipe( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.transposeMapOption( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.fn(function* ([video]) { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const [bucket] = yield* S3Buckets.getBucketAccess(video.bucketId); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const listResponse = yield* bucket.listObjects({ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
prefix: `${video.ownerId}/${video.id}/`, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const contents = listResponse.Contents || []; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
const thumbnailKey = contents.find((item) => | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
item.Key?.endsWith("screen-capture.jpg"), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
)?.Key; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
if (!thumbnailKey) return Option.none(); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
return Option.some( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
yield* bucket.getSignedObjectUrl(thumbnailKey), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Effect.map(Option.flatten), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
Comment on lines
+167
to
+193
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t call After Apply this diff: - getThumbnailURL: Effect.fn("Videos.getThumbnailURL")(function* (
- videoId: Video.VideoId,
- ) {
- const videoOpt = yield* repo
- .getById(videoId)
- .pipe(Policy.withPublicPolicy(policy.canView(videoId)));
-
- return yield* videoOpt.pipe(
- Effect.transposeMapOption(
- Effect.fn(function* ([video]) {
- const [bucket] = yield* S3Buckets.getBucketAccess(video.bucketId);
-
- const listResponse = yield* bucket.listObjects({
- prefix: `${video.ownerId}/${video.id}/`,
- });
- const contents = listResponse.Contents || [];
-
- const thumbnailKey = contents.find((item) =>
- item.Key?.endsWith("screen-capture.jpg"),
- )?.Key;
-
- if (!thumbnailKey) return Option.none();
-
- return Option.some(
- yield* bucket.getSignedObjectUrl(thumbnailKey),
- );
- }),
- ),
- Effect.map(Option.flatten),
- );
- }),
+ getThumbnailURL: Effect.fn("Videos.getThumbnailURL")((videoId) =>
+ repo
+ .getById(videoId)
+ .pipe(
+ Policy.withPublicPolicy(policy.canView(videoId)),
+ Effect.map(([video]) => video),
+ Effect.flatMap((video) =>
+ Effect.gen(function* () {
+ const [bucket] = yield* S3Buckets.getBucketAccess(
+ video.bucketId,
+ );
+
+ const listResponse = yield* bucket.listObjects({
+ prefix: `${video.ownerId}/${video.id}/`,
+ });
+
+ const thumbnailKey = listResponse.Contents?.find((item) =>
+ item.Key?.endsWith("screen-capture.jpg"),
+ )?.Key;
+
+ if (!thumbnailKey) return Option.none<string>();
+
+ const url = yield* bucket.getSignedObjectUrl(thumbnailKey);
+ return Option.some(url);
+ }),
+ ),
+ ),
+ ), 📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @coderabbitai There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🌐 Web query:
💡 Result: Short answer
How it works (concise)
Example
Reference
@Brendonovich, you're absolutely right! I apologize for the incorrect analysis. If Thank you for the clarification. I'll disregard my previous suggestion.
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
}), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
dependencies: [ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Use VideoId type from domain (not Video.VideoId).
Domain exports VideoId at top-level. Update imports and annotations.
Also applies to: 14-14, 51-51
🤖 Prompt for AI Agents