diff --git a/tests/upload-handler.test.ts b/tests/upload-handler.test.ts index 6d2a0a3..324ab9e 100644 --- a/tests/upload-handler.test.ts +++ b/tests/upload-handler.test.ts @@ -416,6 +416,125 @@ describe("UploadHandler", () => { } }); + // Fires a chunked (WebSocket) auto-upload from the input named "doc" inside + // the given form markup and returns whatever console.warn was called with. + const chunkedUploadWarnings = async (formHTML: string): Promise => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + const chunkedHandler = new UploadHandler(mockSendMessage); + + const form = document.createElement("form"); + form.innerHTML = formHTML; + const input = form.querySelector('input[name="doc"]') as HTMLInputElement; + + await chunkedHandler.startUpload( + "doc", + [createMockFile("scan.png", "imgdata", "image/png")], + input + ); + await chunkedHandler.handleUploadStartResponse({ + upload_name: "doc", + entries: [ + { + entry_id: "entry-1", + client_name: "scan.png", + valid: true, + auto_upload: true, + }, + ], + }); + await jest.runAllTimersAsync(); + + const messages = warn.mock.calls.map((c) => String(c[0])); + warn.mockRestore(); + return messages; + }; + + it("warns that marked fields cannot travel on a chunked upload (#508)", async () => { + const warnings = await chunkedUploadWarnings( + '' + + '' + ); + + // The chunked transport carries no form fields, so the mark is silently + // inert — say so rather than leaving an empty value in the handler with + // nothing in the markup to explain it. + const relevant = warnings.filter((m) => m.includes("lvt-upload-with")); + expect(relevant).toHaveLength(1); + expect(relevant[0]).toContain("id"); + expect(relevant[0]).toContain("chunked"); + }); + + it("stays quiet on a chunked upload when no field is marked", async () => { + const warnings = await chunkedUploadWarnings( + '' + + '' + ); + + // Nothing marked means nothing was promised, so there is nothing to warn + // about — a warning on every chunked upload would train people to ignore it. + expect(warnings.filter((m) => m.includes("lvt-upload-with"))).toHaveLength(0); + }); + + it("stays quiet on a proxied upload, where marked fields do travel", async () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + await proxiedUploadFormData( + '' + + '' + ); + + // Proxied is always multipart, so the fields arrive and the warning would + // be plain wrong. + const messages = warn.mock.calls.map((c) => String(c[0])); + warn.mockRestore(); + expect(messages.filter((m) => m.includes("lvt-upload-with"))).toHaveLength(0); + }); + + it("warns on a Direct upload, which never carries fields on any path (#508)", async () => { + const warn = jest.spyOn(console, "warn").mockImplementation(() => {}); + const mockUploader = { upload: jest.fn().mockResolvedValue(undefined) }; + const directHandler = new UploadHandler(mockSendMessage); + directHandler.registerUploader("mock", mockUploader); + + const form = document.createElement("form"); + form.innerHTML = + '' + + ''; + const input = form.querySelector('input[name="doc"]') as HTMLInputElement; + + await directHandler.startUpload( + "doc", + [createMockFile("scan.png", "imgdata", "image/png")], + input + ); + await directHandler.handleUploadStartResponse({ + upload_name: "doc", + entries: [ + { + entry_id: "entry-1", + client_name: "scan.png", + valid: true, + auto_upload: true, + mode: "direct", + external: { uploader: "mock", url: "https://cdn.example/scan.png" }, + }, + ], + }); + await jest.runAllTimersAsync(); + + const messages = warn.mock.calls.map((c) => String(c[0])); + warn.mockRestore(); + + // Direct PUTs straight to storage and completes with a metadata-only + // message, so marked fields reach the handler on NO path — not even with + // the socket down, which is the multipart fallback Volume gets and Direct + // does not. The remedy has to differ accordingly. + const relevant = messages.filter((m) => m.includes("lvt-upload-with")); + expect(relevant).toHaveLength(1); + expect(relevant[0]).toContain("direct-to-storage"); + expect(relevant[0]).toContain("controller state"); + expect(relevant[0]).not.toContain("when the socket is down"); + }); + it("previews locally without uploading when mode is preview", async () => { const createObjectURL = jest .fn() diff --git a/upload/upload-handler.ts b/upload/upload-handler.ts index 4690b4c..a3aad2a 100644 --- a/upload/upload-handler.ts +++ b/upload/upload-handler.ts @@ -358,6 +358,8 @@ export class UploadHandler { entry: UploadEntry, meta: ExternalUploadMeta ): Promise { + this.warnIfMarkedFieldsCannotTravel(entry, "direct-to-storage"); + try { const uploader = this.uploaders.get(meta.uploader); if (!uploader) { @@ -618,6 +620,44 @@ export class UploadHandler { this.previewUrls.clear(); } + /** + * Warn when a form marks fields with lvt-upload-with but the upload is leaving + * by a transport that carries no form fields. Only the multipart POST carries + * them: the chunked WebSocket path sends bytes and entry ids, and the Direct + * path PUTs straight to storage and completes with a metadata-only message — + * in both cases the server builds the action's context from an empty map. + * Without this the fields simply never arrive and the handler sees empty + * values, with nothing in the markup to suggest why. + * + * transport names the path taken, since the remedy differs: a Volume upload + * does carry fields when it falls back to multipart, while Direct never does + * on any path. Tracked as livetemplate/livetemplate#508. + */ + private warnIfMarkedFieldsCannotTravel( + entry: UploadEntry, + transport: "chunked WebSocket" | "direct-to-storage" + ): void { + const form = entry.sourceInput?.form; + if (!form) return; + const marked = Array.from(form.elements) + .filter((el) => el.hasAttribute("lvt-upload-with")) + .map((el) => (el as HTMLInputElement).name) + .filter(Boolean); + if (marked.length === 0) return; + const remedy = + transport === "chunked WebSocket" + ? `Fields travel on the multipart path only — Proxied uploads, and this ` + + `field when the socket is down.` + : `Fields travel on the multipart path only, which Direct never uses; ` + + `read the context from controller state instead.`; + console.warn( + `Upload "${entry.uploadName}" is using the ${transport} transport, which ` + + `does not carry form fields — ${marked.join(", ")} marked lvt-upload-with ` + + `will not reach the upload_${entry.uploadName}_complete handler. ` + + `${remedy} See livetemplate/livetemplate#508.` + ); + } + /** * Upload file in chunks via WebSocket */ @@ -625,6 +665,8 @@ export class UploadHandler { const { file, id } = entry; let offset = 0; + this.warnIfMarkedFieldsCannotTravel(entry, "chunked WebSocket"); + // Create abort controller for cancellation entry.abortController = new AbortController();