From 33573f8c3fc859a0e98f7449d2387c8c96942d5f Mon Sep 17 00:00:00 2001 From: Aditya Garud <153842990+yashranaway@users.noreply.github.com> Date: Thu, 29 Oct 2026 20:45:41 +0000 Subject: [PATCH] fix(web): preserve personal bucket lookup failures --- .../unit/s3-bucket-connections.test.ts | 91 ++++++++++++++++++- packages/web-backend/src/S3Buckets/index.ts | 6 +- 2 files changed, 88 insertions(+), 9 deletions(-) diff --git a/apps/web/__tests__/unit/s3-bucket-connections.test.ts b/apps/web/__tests__/unit/s3-bucket-connections.test.ts index f2d1f5580e7..02a61a7a901 100644 --- a/apps/web/__tests__/unit/s3-bucket-connections.test.ts +++ b/apps/web/__tests__/unit/s3-bucket-connections.test.ts @@ -1,10 +1,17 @@ import { createServer, request as httpRequest } from "node:http"; import type { Socket } from "node:net"; -import { S3Bucket } from "@cap/web-domain"; -import { ConfigProvider, Effect, Layer, ManagedRuntime, Option } from "effect"; +import { DatabaseError, S3Bucket, User } from "@cap/web-domain"; +import { + ConfigProvider, + Effect, + Either, + Layer, + ManagedRuntime, + Option, +} from "effect"; import { describe, expect, it, vi } from "vitest"; -const mocks = vi.hoisted(() => ({ getById: vi.fn() })); +const mocks = vi.hoisted(() => ({ getById: vi.fn(), getForUser: vi.fn() })); vi.mock("@cap/database/crypto", () => ({ decrypt: async (value: string) => value, @@ -34,7 +41,10 @@ vi.mock("@cap/web-backend/src/Aws.ts", async () => { vi.mock("@cap/web-backend/src/S3Buckets/S3BucketsRepo.ts", async () => { const { Effect } = await import("effect"); class S3BucketsRepo extends Effect.Service()("S3BucketsRepo", { - sync: () => ({ getById: mocks.getById }), + sync: () => ({ + getById: mocks.getById, + getForUser: mocks.getForUser, + }), }) {} return { S3BucketsRepo }; }); @@ -267,3 +277,76 @@ describe("S3 connection reuse", () => { } }); }); + +describe("personal bucket selection", () => { + it("propagates lookup failures instead of selecting Cap storage", async () => { + const fixture = await storageFixture(); + const error = new DatabaseError({ cause: new Error("Connection lost") }); + mocks.getForUser.mockReturnValue(Effect.fail(error)); + try { + const result = await fixture.runtime.runPromise( + fixture.service + .getBucketAccessForUser(User.UserId.make("owner")) + .pipe(Effect.either), + ); + expect(result).toEqual(Either.left(error)); + expect(mocks.getForUser).toHaveBeenLastCalledWith("owner"); + } finally { + await fixture.close(); + } + }); + + it("uses Cap storage when the user has no active custom bucket", async () => { + const fixture = await storageFixture(); + mocks.getForUser.mockReturnValue(Effect.succeed(Option.none())); + try { + const [access, bucket] = await fixture.runtime.runPromise( + fixture.service.getBucketAccessForUser(User.UserId.make("owner")), + ); + expect(bucket).toEqual(Option.none()); + const url = new URL( + await fixture.runtime.runPromise( + access.getPresignedPutUrl("recording.mp4"), + ), + ); + expect(url.pathname).toBe("/capso/recording.mp4"); + expect(url.searchParams.get("X-Amz-Credential")).toContain( + "default-key/", + ); + } finally { + await fixture.close(); + } + }); + + it("signs new uploads with the user's active custom bucket", async () => { + const fixture = await storageFixture(); + const customBucket = S3Bucket.decodeSync({ + id: "custom-bucket", + ownerId: "owner", + region: "eu-west-1", + endpoint: fixture.endpoint, + name: "custom-storage", + accessKeyId: "custom-key", + secretAccessKey: "custom-secret", + }); + mocks.getForUser.mockReturnValue(Effect.succeed(Option.some(customBucket))); + try { + const [access, bucket] = await fixture.runtime.runPromise( + fixture.service.getBucketAccessForUser(User.UserId.make("owner")), + ); + expect(bucket).toEqual(Option.some(customBucket)); + const url = new URL( + await fixture.runtime.runPromise( + access.getPresignedPutUrl("recording.mp4"), + ), + ); + expect(url.pathname).toBe("/custom-storage/recording.mp4"); + expect(url.searchParams.get("X-Amz-Credential")).toContain("custom-key/"); + expect(url.searchParams.get("X-Amz-Credential")).toContain( + "/eu-west-1/s3/", + ); + } finally { + await fixture.close(); + } + }); +}); diff --git a/packages/web-backend/src/S3Buckets/index.ts b/packages/web-backend/src/S3Buckets/index.ts index e3edfe307e2..f05fafea696 100644 --- a/packages/web-backend/src/S3Buckets/index.ts +++ b/packages/web-backend/src/S3Buckets/index.ts @@ -189,11 +189,7 @@ export class S3Buckets extends Effect.Service()("S3Buckets", { function* (userId: User.UserId) { return yield* repo .getForUser(userId) - .pipe( - Effect.option, - Effect.map(Option.flatten), - Effect.flatMap(getBucketAccess), - ); + .pipe(Effect.flatMap(getBucketAccess)); }, ), getBucketAccessForOrganization: Effect.fn(