diff --git a/packages/client/src/client.js b/packages/client/src/client.js index 43b21614d..bd492e8b0 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -879,7 +879,24 @@ export class PercyClient { } let snapshot = await this.createSnapshot(buildId, options); let comparison = await this.createComparison(snapshot.data.id, options); - await this.uploadComparisonTiles(comparison.data.id, options.tiles); + try { + await this.uploadComparisonTiles(comparison.data.id, options.tiles); + } catch (error) { + // A tile failed to upload/verify after all retries (see #verify). Finalize + // the comparison anyway so the server resolves it deterministically and + // marks this snapshot failed — an unfinalized comparison is otherwise swept + // by the build-cleanup retry loop and surfaces as a misleading render_timeout. + // Re-throw afterwards so the snapshot is reported as failed to the caller. + this.log.error(`Failed to upload tiles for comparison: ${comparison.data.id} ${options.tag.name}`, meta); + try { + await this.finalizeComparison(comparison.data.id); + } catch (finalizeError) { + // String-coerce the rejection so a non-Error value (null/undefined/string) + // can't throw here and mask the original tile-upload error we re-throw below. + this.log.debug(`Failed to finalize failed comparison ${comparison.data.id}: ${finalizeError}`, meta); + } + throw error; + } this.log.debug(`Created comparison: ${comparison.data.id} ${options.tag.name}`, meta); await this.finalizeComparison(comparison.data.id); this.log.debug(`Finalized comparison: ${comparison.data.id} ${options.tag.name}`, meta); diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index 84f0e3b0a..ad52fc347 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -2413,6 +2413,40 @@ describe('PercyClient', () => { }); }); + describe('when a tile fails to upload after all retries', () => { + it('finalizes the comparison and rethrows so the snapshot is marked failed', async () => { + spyOn(client, 'uploadComparisonTiles').and.callFake(() => + Promise.reject(new Error('Uploading comparison tile failed'))); + spyOn(client, 'finalizeComparison').and.callThrough(); + + await expectAsync(client.sendComparison(123, { + name: 'test snapshot name', + tag: { name: 'test tag' }, + tiles: [{ sha: 'abcd' }] + })).toBeRejectedWithError('Uploading comparison tile failed'); + + // the comparison is still finalized so the server fails it deterministically + expect(client.finalizeComparison).toHaveBeenCalled(); + expect(api.requests['/comparisons/891011/finalize']).toBeDefined(); + }); + + it('still rethrows the original error when finalizing the failed comparison also fails', async () => { + spyOn(client, 'uploadComparisonTiles').and.callFake(() => + Promise.reject(new Error('Uploading comparison tile failed'))); + spyOn(client, 'finalizeComparison').and.callFake(() => + Promise.reject(new Error('finalize boom'))); + + await expectAsync(client.sendComparison(123, { + name: 'test snapshot name', + tag: { name: 'test tag' }, + tiles: [{ sha: 'abcd' }] + })).toBeRejectedWithError('Uploading comparison tile failed'); + + // the finalize attempt was still made (and its own failure swallowed) + expect(client.finalizeComparison).toHaveBeenCalled(); + }); + }); + describe('when labels are provided on a POA comparison', () => { beforeEach(async () => { await client.sendComparison(123, {