-
Notifications
You must be signed in to change notification settings - Fork 1.5k
feat(mail): make mobile attachment sends durable #3404
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
Merged
Merged
Changes from 5 commits
Commits
Show all changes
9 commits
Select commit
Hold shift + click to select a range
7f9d3c5
feat(mail): accept durable multipart attachments
elie222 65db15d
refactor(mail): bound multipart processing memory
elie222 02bad1d
refactor(mail): lazily prepare durable sends
elie222 d2bd205
fix(mail): distinguish durable preparation crashes
elie222 6979407
feat(mail): stage mobile attachments outside function payloads
elie222 e7df685
fix(mail): harden attachment API boundaries
elie222 1f7988c
fix(mail): harden attachment stage reservations
elie222 ef382a3
refactor(mail): simplify attachment staging
elie222 f4eeacc
refactor(mail): remove attachment staging
elie222 File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
94 changes: 94 additions & 0 deletions
94
apps/web/app/api/messages/send-attachments/complete/route.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| import { NextRequest } from "next/server"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { POST } from "./route"; | ||
|
|
||
| const completeEmailAttachments = vi.hoisted(() => vi.fn()); | ||
| const stagingErrors = vi.hoisted(() => ({ | ||
| Consumed: class EmailAttachmentStageConsumedError extends Error {}, | ||
| Incomplete: class EmailAttachmentStageIncompleteError extends Error {}, | ||
| Invalid: class EmailAttachmentStageInvalidError extends Error {}, | ||
| })); | ||
|
|
||
| vi.mock("@/utils/email/email-attachment-staging", () => ({ | ||
| completeEmailAttachments, | ||
| EmailAttachmentStageConsumedError: stagingErrors.Consumed, | ||
| EmailAttachmentStageIncompleteError: stagingErrors.Incomplete, | ||
| EmailAttachmentStageInvalidError: stagingErrors.Invalid, | ||
| })); | ||
| vi.mock("@/utils/middleware", () => ({ | ||
| withEmailAccount: | ||
| (_name: string, handler: (request: MockedRequest) => Promise<Response>) => | ||
| (request: NextRequest) => | ||
| handler( | ||
| Object.assign(request, { | ||
| auth: { emailAccountId: "account-1" }, | ||
| }) as MockedRequest, | ||
| ), | ||
| })); | ||
|
|
||
| type MockedRequest = NextRequest & { auth: { emailAccountId: string } }; | ||
|
|
||
| describe("POST /api/messages/send-attachments/complete", () => { | ||
| beforeEach(() => vi.clearAllMocks()); | ||
|
|
||
| it("verifies opaque IDs inside the authenticated account scope", async () => { | ||
| completeEmailAttachments.mockResolvedValue({ | ||
| attachments: [ | ||
| { id: "attachment-1", stageId: "stage-1", status: "ready" }, | ||
| ], | ||
| }); | ||
| const input = { | ||
| mutationId: "41ec6d2b-d0e8-4f75-924a-f6f4e5bab4cf", | ||
| attachments: [{ id: "attachment-1", stageId: "stage-1" }], | ||
| }; | ||
|
|
||
| const response = await POST( | ||
| new NextRequest( | ||
| "http://localhost/api/messages/send-attachments/complete", | ||
| { | ||
| method: "POST", | ||
| body: JSON.stringify(input), | ||
| headers: { "content-type": "application/json" }, | ||
| }, | ||
| ), | ||
| { params: Promise.resolve({}) }, | ||
| ); | ||
|
|
||
| await expect(response.json()).resolves.toEqual({ | ||
| attachments: [ | ||
| { id: "attachment-1", stageId: "stage-1", status: "ready" }, | ||
| ], | ||
| }); | ||
| expect(completeEmailAttachments).toHaveBeenCalledWith({ | ||
| emailAccountId: "account-1", | ||
| input, | ||
| }); | ||
| }); | ||
|
|
||
| it.each([ | ||
| [new stagingErrors.Incomplete("Upload incomplete"), 409], | ||
| [new stagingErrors.Consumed("Upload expired"), 410], | ||
| [new stagingErrors.Invalid("Upload invalid"), 410], | ||
| ])("maps staging lifecycle failures to a retryable status", async (error, status) => { | ||
| completeEmailAttachments.mockRejectedValue(error); | ||
|
|
||
| const response = await post(); | ||
|
|
||
| expect(response.status).toBe(status); | ||
| await expect(response.json()).resolves.toEqual({ error: error.message }); | ||
| }); | ||
| }); | ||
|
|
||
| function post() { | ||
| return POST( | ||
| new NextRequest("http://localhost/api/messages/send-attachments/complete", { | ||
| method: "POST", | ||
| body: JSON.stringify({ | ||
| mutationId: "41ec6d2b-d0e8-4f75-924a-f6f4e5bab4cf", | ||
| attachments: [{ id: "attachment-1", stageId: "stage-1" }], | ||
| }), | ||
| headers: { "content-type": "application/json" }, | ||
| }), | ||
| { params: Promise.resolve({}) }, | ||
| ); | ||
| } |
34 changes: 34 additions & 0 deletions
34
apps/web/app/api/messages/send-attachments/complete/route.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,34 @@ | ||
| import { NextResponse } from "next/server"; | ||
| import { | ||
| completeEmailAttachments, | ||
| EmailAttachmentStageConsumedError, | ||
| EmailAttachmentStageIncompleteError, | ||
| EmailAttachmentStageInvalidError, | ||
| } from "@/utils/email/email-attachment-staging"; | ||
| import { completeEmailAttachmentsBody } from "@/utils/email/email-attachment-staging.validation"; | ||
| import { withEmailAccount } from "@/utils/middleware"; | ||
|
|
||
| export const POST = withEmailAccount( | ||
| "messages/send-attachments/complete", | ||
| async (request) => { | ||
| const input = completeEmailAttachmentsBody.parse(await request.json()); | ||
| try { | ||
| const result = await completeEmailAttachments({ | ||
| emailAccountId: request.auth.emailAccountId, | ||
| input, | ||
| }); | ||
| return NextResponse.json(result); | ||
| } catch (error) { | ||
| if (error instanceof EmailAttachmentStageIncompleteError) { | ||
| return NextResponse.json({ error: error.message }, { status: 409 }); | ||
| } | ||
| if ( | ||
| error instanceof EmailAttachmentStageInvalidError || | ||
| error instanceof EmailAttachmentStageConsumedError | ||
| ) { | ||
| return NextResponse.json({ error: error.message }, { status: 410 }); | ||
| } | ||
| throw error; | ||
| } | ||
| }, | ||
| ); |
78 changes: 78 additions & 0 deletions
78
apps/web/app/api/messages/send-attachments/stage/route.test.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,78 @@ | ||
| import { NextRequest } from "next/server"; | ||
| import { beforeEach, describe, expect, it, vi } from "vitest"; | ||
| import { POST } from "./route"; | ||
|
|
||
| const staging = vi.hoisted(() => ({ | ||
| stageEmailAttachments: vi.fn(), | ||
| })); | ||
|
|
||
| vi.mock("@/utils/email/email-attachment-staging", () => ({ | ||
| ...staging, | ||
| EmailAttachmentStageConflictError: class EmailAttachmentStageConflictError extends Error {}, | ||
| EmailAttachmentStageUnavailableError: class EmailAttachmentStageUnavailableError extends Error {}, | ||
| })); | ||
| vi.mock("@/utils/middleware", () => ({ | ||
| withEmailAccount: | ||
| (_name: string, handler: (request: MockedRequest) => Promise<Response>) => | ||
| (request: NextRequest) => | ||
| handler( | ||
| Object.assign(request, { | ||
| auth: { emailAccountId: "account-1" }, | ||
| }) as MockedRequest, | ||
| ), | ||
| })); | ||
|
|
||
| type MockedRequest = NextRequest & { auth: { emailAccountId: string } }; | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
|
|
||
| describe("POST /api/messages/send-attachments/stage", () => { | ||
| beforeEach(() => vi.clearAllMocks()); | ||
|
|
||
| it("passes only the authenticated account context to staging", async () => { | ||
| staging.stageEmailAttachments.mockResolvedValue({ mode: "direct" }); | ||
|
|
||
| const response = await post(stageBody()); | ||
|
|
||
| await expect(response.json()).resolves.toEqual({ mode: "direct" }); | ||
| expect(staging.stageEmailAttachments).toHaveBeenCalledWith({ | ||
| emailAccountId: "account-1", | ||
| input: stageBody(), | ||
| }); | ||
| }); | ||
|
|
||
| it("rejects malformed aggregate metadata before creating intents", async () => { | ||
| const body = stageBody(); | ||
| body.attachments[0].size = 20 * 1024 * 1024; | ||
|
|
||
| await expect(post(body)).rejects.toThrow( | ||
| "Attachments must be 10 MB or smaller.", | ||
| ); | ||
| expect(staging.stageEmailAttachments).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| function post(body: unknown) { | ||
| return POST( | ||
| new NextRequest("http://localhost/api/messages/send-attachments/stage", { | ||
| method: "POST", | ||
| body: JSON.stringify(body), | ||
| headers: { "content-type": "application/json" }, | ||
| }), | ||
| { params: Promise.resolve({}) }, | ||
| ); | ||
| } | ||
|
|
||
| function stageBody() { | ||
| return { | ||
| mutationId: "41ec6d2b-d0e8-4f75-924a-f6f4e5bab4cf", | ||
| queuedAt: 1_788_000_000_000, | ||
| attachments: [ | ||
| { | ||
| id: "attachment-1", | ||
| filename: "notes.txt", | ||
| mimeType: "text/plain", | ||
| size: 5, | ||
| disposition: "attachment" as const, | ||
| }, | ||
| ], | ||
| }; | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,30 @@ | ||
| import { NextResponse } from "next/server"; | ||
| import { | ||
| EmailAttachmentStageConflictError, | ||
| EmailAttachmentStageUnavailableError, | ||
| stageEmailAttachments, | ||
| } from "@/utils/email/email-attachment-staging"; | ||
| import { stageEmailAttachmentsBody } from "@/utils/email/email-attachment-staging.validation"; | ||
| import { withEmailAccount } from "@/utils/middleware"; | ||
|
|
||
| export const POST = withEmailAccount( | ||
| "messages/send-attachments/stage", | ||
| async (request) => { | ||
| const input = stageEmailAttachmentsBody.parse(await request.json()); | ||
| try { | ||
| const result = await stageEmailAttachments({ | ||
| emailAccountId: request.auth.emailAccountId, | ||
| input, | ||
| }); | ||
| return NextResponse.json(result); | ||
| } catch (error) { | ||
| if (error instanceof EmailAttachmentStageUnavailableError) { | ||
| return NextResponse.json({ error: error.message }, { status: 503 }); | ||
| } | ||
| if (error instanceof EmailAttachmentStageConflictError) { | ||
| return NextResponse.json({ error: error.message }, { status: 409 }); | ||
| } | ||
| throw error; | ||
| } | ||
| }, | ||
| ); | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.