fix(sdk): don't accept a transcode as done without its cid

packages/sdk/src/sdk/services/Storage/Storage.test.ts

@@ -50,3 +50,81 @@ describe('generatePreview', () => {
     ).rejects.toThrow('status: 401')
   })
 })
+
+// A storage node can answer `done` from an upload row it has not finished
+// replicating, with no transcode results on it yet. Accepting that response
+// writes a track whose trackCid is undefined: the upload "succeeds" into a
+// track that can never be played, with no error raised anywhere.
+describe('pollProcessingStatus', () => {
+  beforeEach(() => {
+    mockFetch.mockReset()
+  })
+
+  const statusResponse = (body: unknown) =>
+    ({ ok: true, json: async () => body }) as unknown as Response
+
+  const poll = (storage: Storage, template: string) =>
+    (
+      storage as unknown as {
+        pollProcessingStatus: (
+          id: string,
+          template: string,
+          total: number
+        ) => Promise<{ results: Record<string, string> }>
+      }
+    ).pollProcessingStatus('upload-1', template, 1)
+
+  const nodeSelector = {
+    getSelectedNode: async () => 'https://node.example.com',
+    triedSelectingAllNodes: () => false
+  } as unknown as StorageNodeSelectorService
+
+  it('keeps polling when a node reports done with no transcode result', async () => {
+    mockFetch
+      .mockResolvedValueOnce(
+        statusResponse({ id: 'upload-1', status: 'done', results: {} })
+      )
+      .mockResolvedValueOnce(
+        statusResponse({
+          id: 'upload-1',
+          status: 'done',
+          results: { '320': 'QmTranscoded' }
+        })
+      )
+
+    const storage = new Storage({ storageNodeSelector: nodeSelector })
+    const resp = await poll(storage, 'audio')
+
+    expect(resp.results['320']).toBe('QmTranscoded')
+    expect(mockFetch).toHaveBeenCalledTimes(2)
+  }, 20000)
+
+  it('returns immediately once the transcode result is present', async () => {
+    mockFetch.mockResolvedValue(
+      statusResponse({
+        id: 'upload-1',
+        status: 'done',
+        results: { '320': 'QmTranscoded' }
+      })
+    )
+
+    const storage = new Storage({ storageNodeSelector: nodeSelector })
+    const resp = await poll(storage, 'audio')
+
+    expect(resp.results['320']).toBe('QmTranscoded')
+    expect(mockFetch).toHaveBeenCalledTimes(1)
+  })
+
+  // Image resizes have no '320' result to wait for; the gate is audio-only.
+  it('does not require a 320 result for image templates', async () => {
+    mockFetch.mockResolvedValue(
+      statusResponse({ id: 'upload-1', status: 'done', results: {} })
+    )
+
+    const storage = new Storage({ storageNodeSelector: nodeSelector })
+    const resp = await poll(storage, 'img_square')
+
+    expect(resp.results).toEqual({})
+    expect(mockFetch).toHaveBeenCalledTimes(1)
+  })
+})

packages/sdk/src/sdk/services/Storage/Storage.ts

@@ -222,6 +222,9 @@ export class Storage implements StorageService {
     const start = Date.now()
     let lastProgressUpdate = Date.now()
     let lastTranscodeProgress = 0
+    // Tracks whether we ever saw a `done` response with no usable transcode
+    // result, so the timeout can say which of the two failures this was.
+    let sawDoneWithoutResults = false
 
     const maxPollingMs =
       template === 'audio'
@@ -254,7 +257,26 @@ export class Storage implements StorageService {
           })
         }
         if (resp?.status === 'done') {
-          return resp
+          // `done` alone is not proof the transcode results are here. Upload
+          // rows replicate across storage nodes, and getProcessingStatus talks
+          // to whichever node is selected, so a mirror can answer `done` from a
+          // row it has not finished catching up on. Accepting that response
+          // hands populateTrackMetadataWithUploadResponse a `results` map with
+          // no '320' key, the track entity gets written with an undefined
+          // trackCid, and the upload succeeds into a track that can never be
+          // played - no error anywhere, just a dead track with a live page.
+          //
+          // Keep polling instead. A node that really is finished will have the
+          // cid on the next pass, and in the genuinely stuck case this times
+          // out loudly rather than silently publishing unplayable audio.
+          if (template === 'audio' && !resp.results?.['320']) {
+            sawDoneWithoutResults = true
+            this.logger.warn(
+              `Storage node reported done with no transcode results, still polling. id=${id}`
+            )
+          } else {
+            return resp
+          }
         }
         if (resp?.status === 'error') {
           throw new Error(
@@ -278,6 +300,11 @@ export class Storage implements StorageService {
       await wait(POLL_STATUS_INTERVAL)
     }
 
+    if (sawDoneWithoutResults) {
+      throw new Error(
+        `Upload reported done but no transcode result appeared within ${maxPollingMs}ms. id=${id}`
+      )
+    }
     throw new Error(`Upload took over ${maxPollingMs}ms. id=${id}`)
   }