diff --git a/packages/sdk/src/sdk/services/Storage/Storage.test.ts b/packages/sdk/src/sdk/services/Storage/Storage.test.ts index 3788eddc8f8..45772c3e1ea 100644 --- a/packages/sdk/src/sdk/services/Storage/Storage.test.ts +++ b/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 }> + } + ).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) + }) +}) diff --git a/packages/sdk/src/sdk/services/Storage/Storage.ts b/packages/sdk/src/sdk/services/Storage/Storage.ts index 40e6271c1ee..d8c6525d913 100644 --- a/packages/sdk/src/sdk/services/Storage/Storage.ts +++ b/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}`) }