From 3d6ffe997b5318b9df25862fcc7b70ff4a864d0d Mon Sep 17 00:00:00 2001 From: thePR0M3TH3AN <53631862+PR0M3TH3AN@users.noreply.github.com> Date: Tue, 16 Jun 2026 20:19:27 -0400 Subject: [PATCH] fix(upload): surface thumbnail/.torrent upload failures to the user MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit These optional-asset uploads are non-fatal, but failures were warn-only — the user got a silently-missing thumbnail or torrent with no signal. Both upload services now emit a 'warning' status at the point of failure and append a note to the final 'Published' status (downgraded to warning variant) listing what failed. Cloudflare-upload audit #5. Test: tests/services/s3-upload-optional-asset-failures.test.mjs. Co-Authored-By: Claude Opus 4.8 --- js/services/r2Service.js | 25 +++- js/services/s3UploadService.js | 26 ++++- ...s3-upload-optional-asset-failures.test.mjs | 108 ++++++++++++++++++ 3 files changed, 156 insertions(+), 3 deletions(-) create mode 100644 tests/services/s3-upload-optional-asset-failures.test.mjs diff --git a/js/services/r2Service.js b/js/services/r2Service.js index dbeac47a..0e100a06 100644 --- a/js/services/r2Service.js +++ b/js/services/r2Service.js @@ -949,6 +949,11 @@ class R2Service { region: effectiveSettings.region, }); + // Track optional-asset failures so the user gets a visible signal instead + // of a silently-missing thumbnail/torrent (Cloudflare-upload #5). + let thumbnailFailed = false; + let torrentFailed = false; + if (thumbnailFile) { this.setCloudflareUploadStatus("Uploading thumbnail...", "info"); const thumbExt = thumbnailFile.name.split('.').pop() || 'jpg'; @@ -970,6 +975,11 @@ class R2Service { } } catch (err) { userLogger.warn("Thumbnail upload failed, continuing with video...", err); + thumbnailFailed = true; + this.setCloudflareUploadStatus( + "Thumbnail upload failed — publishing without it.", + "warning" + ); } } @@ -1009,6 +1019,11 @@ class R2Service { } } catch (err) { userLogger.warn("Torrent metadata upload failed, continuing...", err); + torrentFailed = true; + this.setCloudflareUploadStatus( + "Torrent upload failed — publishing URL-first without the .torrent.", + "warning" + ); } } @@ -1107,9 +1122,15 @@ class R2Service { publishOutcome = Boolean(published); if (publishOutcome) { + const caveats = []; + if (thumbnailFailed) caveats.push("thumbnail"); + if (torrentFailed) caveats.push("torrent"); + const suffix = caveats.length + ? ` (note: ${caveats.join(" & ")} upload failed)` + : ""; this.setCloudflareUploadStatus( - `Published ${publicUrl}`, - "success" + `Published ${publicUrl}${suffix}`, + caveats.length ? "warning" : "success" ); } } diff --git a/js/services/s3UploadService.js b/js/services/s3UploadService.js index 96cb3ca2..5e1ca9e0 100644 --- a/js/services/s3UploadService.js +++ b/js/services/s3UploadService.js @@ -321,6 +321,11 @@ export class S3UploadService { let statusMessage = `Uploading to ${normalized.bucket}…`; this.setUploadStatus(statusMessage, "info"); + // Track optional-asset failures so the user gets a visible signal instead + // of a silently-missing thumbnail/torrent (Cloudflare-upload #5). + let thumbnailFailed = false; + let torrentFailed = false; + if (thumbnailFile) { this.setUploadStatus("Uploading thumbnail...", "info"); const thumbExt = thumbnailFile.name.split(".").pop() || "jpg"; @@ -350,6 +355,11 @@ export class S3UploadService { "Thumbnail upload failed, continuing with video...", err ); + thumbnailFailed = true; + this.setUploadStatus( + "Thumbnail upload failed — publishing without it.", + "warning" + ); } } @@ -404,6 +414,11 @@ export class S3UploadService { "Torrent metadata upload failed, continuing...", err ); + torrentFailed = true; + this.setUploadStatus( + "Torrent upload failed — publishing URL-first without the .torrent.", + "warning" + ); } } @@ -481,7 +496,16 @@ export class S3UploadService { }); if (published) { - this.setUploadStatus(`Published ${publicUrl}`, "success"); + const caveats = []; + if (thumbnailFailed) caveats.push("thumbnail"); + if (torrentFailed) caveats.push("torrent"); + const suffix = caveats.length + ? ` (note: ${caveats.join(" & ")} upload failed)` + : ""; + this.setUploadStatus( + `Published ${publicUrl}${suffix}`, + caveats.length ? "warning" : "success" + ); } return Boolean(published); } catch (err) { diff --git a/tests/services/s3-upload-optional-asset-failures.test.mjs b/tests/services/s3-upload-optional-asset-failures.test.mjs new file mode 100644 index 00000000..77052c6b --- /dev/null +++ b/tests/services/s3-upload-optional-asset-failures.test.mjs @@ -0,0 +1,108 @@ +// Scenario (SCN-upload-optional-asset-failure): +// Thumbnail and .torrent uploads are non-fatal, but their failures used to be +// swallowed (warn-only) so the user got a silently-missing asset +// (Cloudflare-upload audit #5). +// Given a video upload where the thumbnail AND torrent uploads fail, +// When uploadVideo runs and the note still publishes, +// Then the user sees a 'warning' status for each failed asset, and the final +// success status is a 'warning' that names what failed. +// +// Anti-cheat: drives the real uploadVideo control flow with the per-asset upload +// made to fail by content type; asserts the externally observable status events +// (message + variant), not internals. + +import "../test-helpers/setup-localstorage.mjs"; +import test from "node:test"; +import assert from "node:assert/strict"; + +import { S3UploadService } from "../../js/services/s3UploadService.js"; + +function makeDeps() { + return { + ensureS3SdkLoaded: async () => {}, + makeS3Client: () => ({}), + // Fail thumbnail (image/*) and torrent uploads; succeed for the video. + multipartUpload: async ({ contentType }) => { + if ( + contentType && + (contentType.startsWith("image/") || contentType === "application/x-bittorrent") + ) { + throw new Error("simulated optional-asset upload failure"); + } + }, + buildR2Key: () => "u/npub/ns/video.mp4", + computeStorageContentHash: async () => "sha256deadbeef", + buildS3ObjectUrl: () => "https://mock.example/file", + getCorsOrigins: () => ["*"], + prepareS3Connection: async (o) => ({ ...o, bucket: "b", publicBaseUrl: "https://mock.example" }), + validateS3Connection: (s) => ({ + ...s, + endpoint: "https://s3.mock", + region: "auto", + accessKeyId: "k", + secretAccessKey: "s", + bucket: "b", + forcePathStyle: false, + publicBaseUrl: "https://mock.example", + }), + userLogger: { warn() {}, error() {} }, + buildStoragePointerValue: () => "ptr", + buildStoragePrefixFromKey: () => "prefix", + getVideoNoteErrorMessage: (c) => `Error: ${c}`, + normalizeVideoNotePayload: (p) => ({ payload: p, errors: [] }), + calculateTorrentInfoHash: async () => "a".repeat(40), + }; +} + +const blob = (name, type) => ({ name, type, size: 8, slice() { return this; } }); + +test("failed thumbnail + torrent uploads surface as warnings and in the final status", async () => { + const service = new S3UploadService(makeDeps()); + const events = []; + service.on("uploadStatus", (d) => events.push(d)); + + const published = await service.uploadVideo({ + npub: "npub1example", + file: blob("video.mp4", "video/mp4"), + thumbnailFile: blob("thumb.jpg", "image/jpeg"), + torrentFile: blob("video.torrent", "application/x-bittorrent"), + metadata: { title: "Test" }, + settings: { endpoint: "https://s3.mock", accessKeyId: "k", secretAccessKey: "s", bucket: "b" }, + infoHash: "a".repeat(40), + publishVideoNote: async () => true, + }); + + assert.equal(published, true, "publish should still succeed despite optional-asset failures"); + + const warnings = events.filter((e) => e.variant === "warning").map((e) => e.message); + assert.ok(warnings.some((m) => /thumbnail/i.test(m)), `expected a thumbnail warning, got ${JSON.stringify(warnings)}`); + assert.ok(warnings.some((m) => /torrent/i.test(m)), `expected a torrent warning, got ${JSON.stringify(warnings)}`); + + const final = events[events.length - 1]; + assert.equal(final.variant, "warning", "final status should be a warning when assets failed"); + assert.match(final.message, /Published/); + assert.match(final.message, /thumbnail/i); + assert.match(final.message, /torrent/i); +}); + +test("clean upload ends with a plain success status (no caveats)", async () => { + const deps = makeDeps(); + deps.multipartUpload = async () => {}; // everything succeeds + const service = new S3UploadService(deps); + const events = []; + service.on("uploadStatus", (d) => events.push(d)); + + await service.uploadVideo({ + npub: "npub1example", + file: blob("video.mp4", "video/mp4"), + thumbnailFile: blob("thumb.jpg", "image/jpeg"), + metadata: { title: "Test" }, + settings: { endpoint: "https://s3.mock", accessKeyId: "k", secretAccessKey: "s", bucket: "b" }, + infoHash: "a".repeat(40), + publishVideoNote: async () => true, + }); + + const final = events[events.length - 1]; + assert.equal(final.variant, "success"); + assert.doesNotMatch(final.message, /note:/i); +});