diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 05a535a2d..3ccdfefba 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -691,6 +691,9 @@ export class Percy { snapshotPromise[snapshot.name] = new Promise((resolve, reject) => { Object.assign(snapshot, { resolve, reject }); }); + // only the requested snapshot's promise is awaited, so a rejected extra + // (additional snapshots, array bodies) must not surface as unhandled + snapshotPromise[snapshot.name].catch(() => {}); } // push each finished snapshot to the snapshots queue this.#snapshots.push(snapshot); @@ -708,6 +711,9 @@ export class Percy { throw new Error('Not running'); } else if (Array.isArray(options)) { return yieldAll(options.map(o => this.yield.upload(o))); + } else if (this.build?.error && callback && this.syncMode(options)) { + // a sync caller waits on the result, so fail it now instead of queueing it + throw new Error(this.build.error); } // validate comparison uploads and warn about any errors diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index de04f2118..ad3d291f0 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -420,6 +420,9 @@ export async function uploadSnapshotLog(percy, buildId, snapshotId, meta = {}) { } } +// What a sync caller is told when its upload never ran and the build has not failed +const SYNC_CANCELLED = 'Snapshot upload was cancelled before it finished, because Percy stopped or a newer snapshot with the same name replaced it'; + // Creates a snapshots queue that manages a Percy build and uploads snapshots. export function createSnapshotsQueue(percy) { let { concurrency } = percy.config.discovery; @@ -542,7 +545,8 @@ export function createSnapshotsQueue(percy) { // Pushing to syncQueue, that will check for // snapshot processing status, and will resolve once done - if (snapshot.sync) { + // (only when someone waits on it: a job without callbacks would throw in the poller) + if (snapshot.sync && snapshot.resolve) { percy.log.info(`Waiting for snapshot '${name}' to be completed`, meta); const data = new JobData(response.data.id, null, snapshot.resolve, snapshot.reject); percy.syncQueue.push(data); @@ -555,8 +559,12 @@ export function createSnapshotsQueue(percy) { let result = { ...snapshot, error }; let { name, meta } = snapshot; - if (error.name === 'QueueClosedError') return result; - if (error.name === 'AbortError') return result; + // a closed or aborted queue never runs the task, so settle any sync caller + // here or the SDK request waiting on it hangs until its own timeout + if (error.name === 'QueueClosedError' || error.name === 'AbortError') { + if (snapshot.sync) snapshot.reject?.(new Error(build?.error || SYNC_CANCELLED)); + return result; + } let failed = error.response?.statusCode === 422 && ( error.response.body.errors.find(e => ( @@ -572,8 +580,10 @@ export function createSnapshotsQueue(percy) { let errors = error.response?.body?.errors; let duplicate = errors?.length > 1 && errors[1].detail.includes('must be unique'); if (duplicate) { + let errMsg = `Ignored duplicate snapshot. ${errors[1].detail}`; + // no comparison was created, so a sync caller has nothing to wait for + if (snapshot.sync) snapshot.reject?.(new Error(errMsg)); if (process.env.PERCY_IGNORE_DUPLICATES !== 'true') { - let errMsg = `Ignored duplicate snapshot. ${errors[1].detail}`; percy.log.warn(errMsg, meta); await percy.suggestionsForFix(errMsg, { snapshotLevel: true, snapshotName: name }); @@ -590,7 +600,7 @@ export function createSnapshotsQueue(percy) { { message: error?.message, meta } ]; await percy.suggestionsForFix(snapshotErrors, { snapshotLevel: true, snapshotName: name }); - if (snapshot.sync) snapshot.reject(error); + if (snapshot.sync) snapshot.reject?.(error); return result; }); } diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 262d323c3..c4393a410 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -1305,6 +1305,181 @@ describe('Percy', () => { ]); }); + it('does not wait on sync snapshot comparisons nobody is waiting for', async () => { + await percy.start(); + + // e.g. the comparison upload route, which passes no callbacks + await percy.upload({ + name: 'Snapshot', + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + }); + + await percy.idle(); + expect(api.requests['/comparisons/891011/finalize']).toHaveSize(1); + expect(percy.syncQueue.jobs).toHaveSize(0); + expect(logger.stderr).toEqual([]); + }); + + it('does not throw for failed sync uploads nobody is waiting for', async () => { + api.reply('/snapshots/4567/comparisons', () => [422, { + errors: [{ detail: 'Invalid comparison' }] + }]); + + await percy.start(); + + await expectAsync(percy.upload({ + name: 'Snapshot', + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + })).toBeResolved(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Encountered an error uploading snapshot: Snapshot' + ])); + expect(logger.stderr.join()).not.toContain('TypeError'); + }); + + it('rejects sync uploads once the build has failed upstream', async () => { + api.reply('/snapshots/4567/comparisons', () => [422, { + errors: [ + { detail: 'Cannot create snapshot in failed builds' }, + { detail: 'Build has failed', source: { pointer: '/data/attributes/build' } } + ] + }]); + + await percy.start(); + + let upload = name => new Promise((resolve, reject) => { + percy.upload({ + name, + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + }, { resolve, reject }).catch(reject); + }); + + // the upload the API rejects, then one sent after the build has failed + await expectAsync(upload('Snapshot 1')).toBeRejectedWithError('Build has failed'); + await expectAsync(upload('Snapshot 2')).toBeRejectedWithError('Build has failed'); + }); + + it('rejects sync uploads the API ignores as duplicates', async () => { + api.reply('/snapshots/4567/comparisons', () => [400, { + errors: [{ status: 'bad_request' }, + { detail: 'The name of each snapshot must be unique ...' }] + }]); + + await percy.start(); + + await expectAsync(new Promise((resolve, reject) => { + percy.upload({ + name: 'Snapshot', + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + }, { resolve, reject }).catch(reject); + })).toBeRejectedWithError( + 'Ignored duplicate snapshot. The name of each snapshot must be unique ...' + ); + }); + + it('does not reject non-sync uploads once the build has failed upstream', async () => { + api.reply('/snapshots/4567/comparisons', () => [422, { + errors: [ + { detail: 'Build has failed', source: { pointer: '/data/attributes/build' } } + ] + }]); + + await percy.start(); + + let upload = name => percy.upload({ + name, + tag: { name: 'device' }, + tiles: [{ content: 'foo' }] + }); + + await expectAsync(upload('Snapshot 1')).toBeResolved(); + expect(percy.build.error).toEqual('Build has failed'); + await expectAsync(upload('Snapshot 2')).toBeResolved(); + + // sync without callbacks (e.g. the comparison upload route) nobody is waiting on + await expectAsync(percy.upload({ + name: 'Snapshot 3', + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + })).toBeResolved(); + }); + + it('rejects queued sync uploads when the build fails while they wait', async () => { + api.reply('/snapshots/4567/comparisons', req => { + // hold the failure so the other uploads queue behind it + req.delay = 500; + return [422, { + errors: [ + { detail: 'Build has failed', source: { pointer: '/data/attributes/build' } } + ] + }]; + }); + + await percy.start(); + + let upload = (name, callback = true) => new Promise((resolve, reject) => { + percy.upload({ + name, + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + }, callback ? { resolve, reject } : null).then(resolve, reject); + }); + + // with a concurrency of 1, the first failure closes the queue while the rest + // wait (the next one may already be dequeued by then); every sync caller, + // sent or aborted while queued, must reject with the build error + let failed = [upload('Snapshot 1'), upload('Snapshot 2')]; + let noCallback = upload('Snapshot 3', false); + let queued = upload('Snapshot 4'); + + await Promise.all([ + ...[...failed, queued].map(u => expectAsync(u).toBeRejectedWithError('Build has failed')), + // a queued sync upload without callbacks is aborted without throwing + expectAsync(noCallback).toBeResolved() + ]); + // the uploads queued behind the first two (Snapshots 3 and 4) never reached the API + expect(api.requests['/snapshots/4567/comparisons'].length).toBeLessThan(3); + }); + + it('rejects queued sync uploads when percy is force stopped', async () => { + api.reply('/snapshots/4567/comparisons', req => { + // hold the first upload so the next one stays queued + req.delay = 500; + return [201, { data: { id: '891011' } }]; + }); + + await percy.start(); + + let upload = name => new Promise((resolve, reject) => { + percy.upload({ + name, + tag: { name: 'device' }, + tiles: [{ content: 'foo' }], + sync: true + }, { resolve, reject }).catch(reject); + }); + + upload('Snapshot 1').catch(() => {}); + let queued = upload('Snapshot 2'); + + let aborted = expectAsync(queued).toBeRejectedWithError( + 'Snapshot upload was cancelled before it finished, because Percy stopped or a newer snapshot with the same name replaced it' + ); + await percy.stop(true); + await aborted; + }); + it('should raise warning in case of upload command with sync', async () => { percy = new Percy({ token: 'PERCY_TOKEN',