diff --git a/packages/cli-exec/src/start.js b/packages/cli-exec/src/start.js index 8b13f6235..d92d324c8 100644 --- a/packages/cli-exec/src/start.js +++ b/packages/cli-exec/src/start.js @@ -19,10 +19,10 @@ export const start = command('start', { log.debug('Skipping percy project attribute calculation'); } - // start percy - yield* percy.yield.start(); - try { + // start percy + yield* percy.yield.start(); + // run until stopped or terminated yield* yieldFor(() => percy.readyState >= 3); } catch (error) { diff --git a/packages/cli-exec/test/exec.test.js b/packages/cli-exec/test/exec.test.js index 31314f45f..1280b0c6b 100644 --- a/packages/cli-exec/test/exec.test.js +++ b/packages/cli-exec/test/exec.test.js @@ -75,7 +75,15 @@ describe('percy exec', () => { it('starts and stops the percy process around the command', async () => { await exec(['--', 'node', '--eval', '']); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); + expect(logger.stdout).toEqual([ '[percy] Percy has started!', '[percy] Running "node --eval "', @@ -126,7 +134,14 @@ describe('percy exec', () => { exec(['--', 'node', '--eval', 'process.exit(3)']) ).toBeRejectedWithError('EEXIT: 3'); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', '[percy] Running "node --eval process.exit(3)"', @@ -139,7 +154,14 @@ describe('percy exec', () => { await exec(['--', 'echo', 'Hi!']); expect(stdoutSpy).toHaveBeenCalled(); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', '[percy] Running "echo Hi!"', @@ -154,9 +176,16 @@ describe('percy exec', () => { exec(['--', 'node', './test/test-data/test_prog.js', 'error']) // Throws Error ).toBeRejectedWithError('EEXIT: 1'); - expect(logger.stderr).toEqual([ + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/', '[percy] Notice: Percy collects CI logs for service improvement, stored for 30 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false' - ]); + ])); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', '[percy] Running "node ./test/test-data/test_prog.js error"', @@ -216,9 +245,15 @@ describe('percy exec', () => { await expectAsync(exec(['--', 'foobar'])).toBeRejected(); expect(stdinSpy).toHaveBeenCalled(); - expect(logger.stderr).toEqual([ - '[percy] Error: spawn error' - ]); + console.log(logger.stderr); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', '[percy] Running "foobar"', @@ -244,7 +279,15 @@ describe('percy exec', () => { // user termination is not considered an error await expectAsync(test).toBeResolved(); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); + expect(logger.stdout).toContain( '[percy] Stopping percy...' ); @@ -259,7 +302,15 @@ describe('percy exec', () => { 'await request(url).catch(e => (console.error(e), process.exit(2)));' ].join('')]); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', jasmine.stringMatching('\\[percy] Running "node '), @@ -272,7 +323,14 @@ describe('percy exec', () => { 'process.env.PERCY_BUILD_ID === "123" || process.exit(2)' )]); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', jasmine.stringMatching('\\[percy] Running "node '), @@ -285,7 +343,14 @@ describe('percy exec', () => { 'process.env.PERCY_BUILD_URL === "https://percy.io/test/test/123" || process.exit(2)' )]); - expect(logger.stderr).toEqual([]); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', jasmine.stringMatching('\\[percy] Running "node '), diff --git a/packages/cli-exec/test/start.test.js b/packages/cli-exec/test/start.test.js index 24719c179..7b0949e31 100644 --- a/packages/cli-exec/test/start.test.js +++ b/packages/cli-exec/test/start.test.js @@ -1,5 +1,5 @@ import { request } from '@percy/cli-command/utils'; -import { logger, setupTest } from '@percy/cli-command/test/helpers'; +import { logger, setupTest, api } from '@percy/cli-command/test/helpers'; import { start, ping } from '@percy/cli-exec'; describe('percy exec:start', () => { @@ -91,9 +91,24 @@ describe('percy exec:start', () => { await expectAsync(start()).toBeRejected(); - expect(logger.stdout).toEqual([]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ + "[percy] Build's CLI and CI logs sent successfully. Please share this log ID with Percy team in case of any issues - random_sha" + ])); + + let lastReq = api.requests['/suggestions/from_logs'].length - 1; + + expect(api.requests['/suggestions/from_logs'][lastReq].body).toEqual({ + data: { + logs: [ + { message: 'Percy is already running or the port 5338 is in use' } + ] + } + }); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ - '[percy] Error: Percy is already running or the port is in use' + '[percy] Notice: Percy collects CI logs for service improvement, stored for 30 days. Opt-out anytime with export PERCY_CLIENT_ERROR_LOGS=false', + '[percy] Error: Percy is already running or the port 5338 is in use', + '[percy] Error: Percy is already running or the port 5338 is in use' ])); }); diff --git a/packages/cli-upload/test/upload.test.js b/packages/cli-upload/test/upload.test.js index 58f705a25..db023c90d 100644 --- a/packages/cli-upload/test/upload.test.js +++ b/packages/cli-upload/test/upload.test.js @@ -199,8 +199,15 @@ describe('percy upload', () => { await up; expect(logger.stderr).toEqual([ - '[percy] AbortError: SIGTERM' + '[percy] AbortError: SIGTERM', + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' ]); + expect(logger.stdout).toEqual(jasmine.arrayContaining([ '[percy] Percy has started!', '[percy] Uploading 3 snapshots...', diff --git a/packages/client/src/client.js b/packages/client/src/client.js index dc237595f..5344b7906 100644 --- a/packages/client/src/client.js +++ b/packages/client/src/client.js @@ -13,6 +13,7 @@ import { getPackageJSON, waitForTimeout, validateTiles, + formatLogErrors, tagsList } from './utils.js'; @@ -544,8 +545,12 @@ export class PercyClient { while (retries > 0 && !success); if (!success) { - this.log.error('Uploading comparison tile failed'); - return false; + let errMsg = 'Uploading comparison tile failed'; + + // Detecting error and logging fix for the same + // We are throwing this error as the comparison will be failed + // even if 1 tile gets failed + throw new Error(errMsg); } return true; } @@ -617,6 +622,15 @@ export class PercyClient { }); } + async getErrorAnalysis(errors) { + const errorLogs = formatLogErrors(errors); + this.log.debug('Sending error logs for analysis'); + + return this.post('suggestions/from_logs', { + data: errorLogs + }); + } + mayBeLogUploadSize(contentSize) { if (contentSize >= 25 * 1024 * 1024) { this.log.error('Uploading resource above 25MB might fail the build...'); diff --git a/packages/client/src/proxy.js b/packages/client/src/proxy.js index d58dda064..9081da2d6 100644 --- a/packages/client/src/proxy.js +++ b/packages/client/src/proxy.js @@ -169,6 +169,14 @@ export class ProxyHttpsAgent extends https.Agent { let handleError = err => { socket.destroy(err); logger('client:proxy').error(`Proxying request failed: ${err}`); + + // We don't get statusCode here, relying on checking error message only + if (!!err.message && (err.message?.includes('ECONNREFUSED') || err.message?.includes('EHOSTUNREACH'))) { + logger('client:proxy').warn('If needed, Please verify if your proxy credentials are correct'); + logger('client:proxy').warn('Please check if your proxy is set correctly and reachable'); + } + + logger('client:proxy').warn('Please check network connection, proxy and ensure that following domains are whitelisted: github.com, percy.io, storage.googleapis.com. In case you are an enterprise customer make sure to whitelist "percy-enterprise.browserstack.com" as well.'); callback(err); }; diff --git a/packages/client/src/utils.js b/packages/client/src/utils.js index 88385a90c..bc2b41d60 100644 --- a/packages/client/src/utils.js +++ b/packages/client/src/utils.js @@ -213,6 +213,26 @@ export function validateTiles(tiles) { return true; } +export function formatLogErrors(errorLogs) { + let errors = []; + if (typeof errorLogs === 'string') { + errors.push({ + message: errorLogs + }); + } else if (Array.isArray(errorLogs)) { + errors = errorLogs; + } else { + errors.push({ + message: errorLogs + }); + errors.push({ + message: errorLogs?.message || '' + }); + } + + return { logs: errors }; +} + // convert tags comma-separated-names to array of objects for POST request export function tagsList(tags) { let tagsArr = []; @@ -226,6 +246,7 @@ export function tagsList(tags) { export { hostnameMatches, + getProxy, ProxyHttpAgent, ProxyHttpsAgent, proxyAgentFor diff --git a/packages/client/test/client.test.js b/packages/client/test/client.test.js index 338faba13..03fed3711 100644 --- a/packages/client/test/client.test.js +++ b/packages/client/test/client.test.js @@ -1303,16 +1303,14 @@ describe('PercyClient', () => { ]); }); - it('returns false if tile is not verified', async () => { + it('throws error if tile is not verified', async () => { api.reply('/comparisons/891011/tiles/verify', async () => { return [400, 'failure']; }); await expectAsync(client.uploadComparisonTiles(891011, [ { sha: sha256hash('foo') } - ])).toBeResolvedTo([ - false - ]); + ])).toBeRejectedWithError('Uploading comparison tile failed'); }); it('throws any errors from verifying', async () => { @@ -1348,6 +1346,10 @@ describe('PercyClient', () => { }); it('verify a comparison tile', async () => { + api.reply('/comparisons/123/tiles/verify', async () => { + return [200, 'success']; + }); + await expectAsync(client.verify(123, 'sha')).toBeResolved(); expect(api.requests['/comparisons/123/tiles/verify']).toBeDefined(); @@ -1572,6 +1574,44 @@ describe('PercyClient', () => { }); }); + describe('#getErrorAnalysis', () => { + const sharedTest = (reqBody, expectedBody) => { + it('should send error logs to API', async () => { + await expectAsync(client.getErrorAnalysis(reqBody)).toBeResolved(); + + expect(api.requests['/suggestions/from_logs']).toBeDefined(); + expect(api.requests['/suggestions/from_logs'][0].method).toBe('POST'); + expect(api.requests['/suggestions/from_logs'][0].body).toEqual({ + data: { + logs: expectedBody + } + }); + }); + }; + describe('when error is of type array', () => { + let body = [ + { message: 'Some Error Log' }, + { message: 'Some Error logs 2' } + ]; + + // Here requestedBody and expectedBody should be same + sharedTest(body, body); + }); + + describe('when error is of type string', () => { + sharedTest('some error', [{ message: 'some error' }]); + }); + + describe('when error is of type object', () => { + sharedTest({ + some_key: 'some_error' + }, [ + { message: { some_key: 'some_error' } }, + { message: '' } + ]); + }); + }); + describe('#mayBeLogUploadSize', () => { it('does not warns when upload size less 20MB/25MB', () => { client.mayBeLogUploadSize(1000); diff --git a/packages/client/test/unit/request.test.js b/packages/client/test/unit/request.test.js index e00879ecf..bd44fddee 100644 --- a/packages/client/test/unit/request.test.js +++ b/packages/client/test/unit/request.test.js @@ -4,6 +4,7 @@ import http from 'http'; import https from 'https'; import { request, ProxyHttpAgent, formatBytes } from '@percy/client/utils'; import { port, href, proxyAgentFor } from '../../src/proxy.js'; +import logger from '@percy/logger/test/helpers'; const ssl = { cert: fs.readFileSync('./test/certs/test.crt'), @@ -272,7 +273,7 @@ describe('Unit / Request', () => { beforeEach(async () => { await server?.close(); - + await logger.mock({ level: 'debug' }); server = await createTestServer({ type: serverType, port: 8080 @@ -473,6 +474,26 @@ describe('Unit / Request', () => { await expectAsync(server.request('/test')) .toBeRejectedWith(error); }); + + if (proxyType === 'https') { + it('throws EHOSTUNREACH error', async () => { + let error = new Error('EHOSTUNREACH'); + + // sabotage the underlying socket.write method to emit an error + spyOn(net.Socket.prototype, 'write') + .and.callFake(function() { + this.emit('error', error); + }); + + await expectAsync(server.request('/test')) + .toBeRejectedWith(error); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy:client:proxy] If needed, Please verify if your proxy credentials are correct', + '[percy:client:proxy] Please check if your proxy is set correctly and reachable' + ])); + }); + } } it('throws an error when unable to connect', async () => { diff --git a/packages/core/src/discovery.js b/packages/core/src/discovery.js index 1ba027156..814a836c9 100644 --- a/packages/core/src/discovery.js +++ b/packages/core/src/discovery.js @@ -289,6 +289,9 @@ export function createDiscoveryQueue(percy) { // on start, launch the browser and run the queue .handle('start', async () => { cache = percy[RESOURCE_CACHE_KEY] = new Map(); + + // If browser.launch() fails it will get captured in + // *percy.start() await percy.browser.launch(); queue.run(); }) @@ -359,15 +362,29 @@ export function createDiscoveryQueue(percy) { }); }); }) - .handle('error', ({ name, meta }, error) => { + .handle('error', async ({ name, meta }, error) => { if (error.name === 'AbortError' && queue.readyState < 3) { // only error about aborted snapshots when not closed - percy.log.error('Received a duplicate snapshot, ' + ( - `the previous snapshot was aborted: ${snapshotLogName(name, meta)}`), meta); + let errMsg = 'Received a duplicate snapshot, ' + ( + `the previous snapshot was aborted: ${snapshotLogName(name, meta)}`); + percy.log.error(errMsg, { snapshotLevel: true, snapshotName: name }); + + await percy.suggestionsForFix(errMsg, meta); } else { // log all other encountered errors - percy.log.error(`Encountered an error taking snapshot: ${name}`, meta); + let errMsg = `Encountered an error taking snapshot: ${name}`; + percy.log.error(errMsg, meta); percy.log.error(error, meta); + + let assetDiscoveryErrors = [ + { message: errMsg, meta }, + { message: error?.message, meta } + ]; + + await percy.suggestionsForFix( + assetDiscoveryErrors, + { snapshotLevel: true, snapshotName: name } + ); } }); } diff --git a/packages/core/src/percy.js b/packages/core/src/percy.js index 9f05f9c30..f1449cf9c 100644 --- a/packages/core/src/percy.js +++ b/packages/core/src/percy.js @@ -1,6 +1,7 @@ import PercyClient from '@percy/client'; import PercyConfig from '@percy/config'; import logger from '@percy/logger'; +import { getProxy } from '@percy/client/utils'; import Browser from './browser.js'; import Pako from 'pako'; import { @@ -86,6 +87,7 @@ export class Percy { if (testing) loglevel = 'silent'; if (loglevel) this.loglevel(loglevel); + this.port = port; this.projectType = projectType; this.testing = testing ? {} : null; this.dryRun = !!testing || !!dryRun; @@ -188,8 +190,11 @@ export class Percy { // throw an easier-to-understand error when the port is in use if (error.code === 'EADDRINUSE') { - throw new Error('Percy is already running or the port is in use'); + let errMsg = `Percy is already running or the port ${this.port} is in use`; + await this.suggestionsForFix(errMsg); + throw new Error(errMsg); } else { + await this.suggestionsForFix(error.message); throw error; } } @@ -281,6 +286,9 @@ export class Percy { this.log.error(err); throw err; } finally { + // This issue doesn't comes under regular error logs, + // it's detected if we just and stop percy server + await this.checkForNoSnapshotCommandError(); await this.sendBuildLogs(); } } @@ -400,6 +408,8 @@ export class Percy { return yield* yieldTo(this.#snapshots.push(options)); } catch (error) { this.#snapshots.cancel(options); + // Detecting and suggesting fix for errors; + await this.suggestionsForFix(error.message); throw error; } }.call(this)); @@ -432,12 +442,89 @@ export class Percy { return syncMode; } + // This specific error will be hard coded + async checkForNoSnapshotCommandError() { + let isPercyStarted = false; + let containsSnapshotTaken = false; + logger.query((item) => { + isPercyStarted ||= item?.message?.includes('Percy has started'); + containsSnapshotTaken ||= item?.message?.includes('Snapshot taken'); + + // This case happens when you directly upload it using cli-upload + containsSnapshotTaken ||= item?.message?.includes('Snapshot uploaded'); + return item; + }); + + if (isPercyStarted && !containsSnapshotTaken) { + // This is the case for No snapshot command called + this.#displaySuggestionLogs([{ + failure_reason: 'Snapshot command was not called', + reason_message: 'Snapshot Command was not called. please check your CI for errors', + suggestion: 'Try using percy snapshot command to take snapshots', + reference_doc_link: ['https://www.browserstack.com/docs/percy/take-percy-snapshots/'] + }]); + } + } + + #displaySuggestionLogs(suggestions, options = {}) { + if (!suggestions?.length) return; + + suggestions.forEach(item => { + const failure = item?.failure_reason; + const failureReason = item?.reason_message; + const suggestion = item?.suggestion; + const referenceDocLinks = item?.reference_doc_link; + + if (options?.snapshotLevel) { + this.log.warn(`Detected erorr for Snapshot: ${options?.snapshotName}`); + } else { + this.log.warn('Detected error for percy build'); + } + + this.log.warn(`Failure: ${failure}`); + this.log.warn(`Failure Reason: ${failureReason}`); + this.log.warn(`Suggestion: ${suggestion}`); + + if (referenceDocLinks?.length > 0) { + this.log.warn('Refer to the below Doc Links for the same'); + + referenceDocLinks?.forEach(_docLink => { + this.log.warn(`* ${_docLink}`); + }); + } + }); + } + + #proxyEnabled() { + return !!(getProxy({ protocol: 'https:' }) || getProxy({})); + } + + async suggestionsForFix(errors, options = {}) { + try { + const suggestionResponse = await this.client.getErrorAnalysis(errors); + this.#displaySuggestionLogs(suggestionResponse, options); + } catch (e) { + // Common error code for Proxy issues + const PROXY_CODES = ['ECONNREFUSED', 'ECONNRESET', 'EHOSTUNREACH']; + if (!!e.code && PROXY_CODES.includes(e.code)) { + // This can be due to proxy issue + this.log.error('percy.io might not be reachable, check network connection, proxy and ensure that percy.io is whitelisted.'); + if (!this.#proxyEnabled()) { + this.log.error('If inside a proxied envirnment, please configure the following environment variables: HTTP_PROXY, [ and optionally HTTPS_PROXY if you need it ]. Refer to our documentation for more details'); + } + } + this.log.error('Unable to analyze error logs'); + this.log.debug(e); + } + } + async sendBuildLogs() { if (!process.env.PERCY_TOKEN) return; try { const logsObject = { clilogs: logger.query(log => !['ci'].includes(log.debug)) }; + // Only add CI logs if not disabled voluntarily. const sendCILogs = process.env.PERCY_CLIENT_ERROR_LOGS !== 'false'; if (sendCILogs) { diff --git a/packages/core/src/snapshot.js b/packages/core/src/snapshot.js index 23dc184ee..d311506af 100644 --- a/packages/core/src/snapshot.js +++ b/packages/core/src/snapshot.js @@ -219,6 +219,7 @@ export async function handleSyncJob(jobPromise, percy, type) { data = await percy.client.getComparisonDetails(id); } } catch (e) { + await percy.suggestionsForFix(e.message); data = { error: e.message }; } return data; @@ -391,7 +392,7 @@ export function createSnapshotsQueue(percy) { return { ...snapshot, response }; }) // handle possible build errors returned by the API - .handle('error', (snapshot, error) => { + .handle('error', async (snapshot, error) => { let result = { ...snapshot, error }; let { name, meta } = snapshot; @@ -413,13 +414,23 @@ export function createSnapshotsQueue(percy) { let duplicate = errors?.length > 1 && errors[1].detail.includes('must be unique'); if (duplicate) { if (process.env.PERCY_IGNORE_DUPLICATES !== 'true') { - percy.log.warn(`Ignored duplicate snapshot. ${errors[1].detail}`); + let errMsg = `Ignored duplicate snapshot. ${errors[1].detail}`; + percy.log.warn(errMsg); + + await percy.suggestionsForFix(errMsg, { snapshotLevel: true, snapshotName: name }); } return result; } - percy.log.error(`Encountered an error uploading snapshot: ${name}`, meta); + let errMsg = `Encountered an error uploading snapshot: ${name}`; + percy.log.error(errMsg, meta); percy.log.error(error, meta); + + let snapshotErrors = [ + { message: errMsg, meta }, + { message: error?.message, meta } + ]; + await percy.suggestionsForFix(snapshotErrors, { snapshotLevel: true, snapshotName: name }); if (snapshot.sync) snapshot.reject(error); return result; }); diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index a0d50d14a..e0acf05c6 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -25,6 +25,12 @@ describe('Discovery', () => { // http://png-pixel.com/ const pixel = Buffer.from('R0lGODlhAQABAIAAAP///wAAACH5BAEAAAAALAAAAAABAAEAAAICRAEAOw==', 'base64'); + const sharedExpectBlock = (expectedBody) => { + let lastReq = api.requests['/suggestions/from_logs'].length - 1; + expect(api.requests['/suggestions/from_logs'][lastReq].body) + .toEqual(expectedBody); + }; + beforeEach(async () => { captured = []; await setupTest(); @@ -923,6 +929,22 @@ describe('Discovery', () => { expect(logger.stderr).toContain( '[percy] Error: Timed out waiting for network requests to idle.' ); + + let expectedRequestBody = { + data: { + logs: [ + { + message: 'Encountered an error taking snapshot: test idle', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'test idle' } } + }, + { + message: 'Timed out waiting for network requests to idle.', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'test idle' } } + } + ] + } + }; + sharedExpectBlock(expectedRequestBody); }); it('shows debug info when requests fail to idle in time', async () => { @@ -941,6 +963,21 @@ describe('Discovery', () => { '', '(?(.|\n)*)$' ].join('\n'))); + + sharedExpectBlock({ + data: { + logs: [ + { + message: 'Encountered an error taking snapshot: test idle', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'test idle' } } + }, + { + message: 'Timed out waiting for network requests to idle.\n\n Active requests:\n - http://localhost:8000/img.gif\n', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'test idle' } } + } + ] + } + }); }); it('shows a warning when idle wait timeout is set over 60000ms', async () => { @@ -1073,6 +1110,21 @@ describe('Discovery', () => { '[percy] Encountered an error taking snapshot: test navigation timeout', '[percy] Error: Navigation failed: Timed out waiting for the page load event' ])); + + sharedExpectBlock({ + data: { + logs: [ + { + message: 'Encountered an error taking snapshot: test navigation timeout', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'test navigation timeout' } } + }, + { + message: 'Navigation failed: Timed out waiting for the page load event', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'test navigation timeout' } } + } + ] + } + }); }); }); @@ -1111,6 +1163,22 @@ describe('Discovery', () => { '', '(?(.|\n)*)$' ].join('\n'))); + + let lastReq = api.requests['/suggestions/from_logs'].length - 1; + expect(api.requests['/suggestions/from_logs'][lastReq].body).toEqual({ + data: { + logs: [ + { + message: 'Encountered an error taking snapshot: navigation idle', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'navigation idle' } } + }, + { + message: 'Navigation failed: Timed out waiting for the page load event\n\n Active requests:\n - http://localhost:8000/img.gif\n', + meta: { build: { id: '123', url: 'https://percy.io/test/test/123', number: 1 }, snapshot: { name: 'navigation idle' } } + } + ] + } + }); }); it('shows a warning when page load timeout is set over 60000ms', async () => { @@ -1846,6 +1914,13 @@ describe('Discovery', () => { })).toBeRejectedWithError( /Failed to launch browser/ ); + + // We are checking here like this, to avoid flaky test as + // the error message contains some number + // eg: `Failed to launch browser. \n[0619/152313.736334:ERROR:command_line_handler.cc(67)` + let lastRequest = api.requests['/suggestions/from_logs'].length - 1; + expect(api.requests['/suggestions/from_logs'][lastRequest].body.data.logs[0].message.includes('Failed to launch browser')) + .toEqual(true); }); it('should fail to launch after the timeout', async () => { @@ -1860,6 +1935,16 @@ describe('Discovery', () => { })).toBeRejectedWithError( /Failed to launch browser\. Timed out after 10ms/ ); + + let expectedBody = { + data: { + logs: [ + { message: "Failed to launch browser. Timed out after 10ms\n'\n\n" } + ] + } + }; + + sharedExpectBlock(expectedBody); }); }); diff --git a/packages/core/test/percy.test.js b/packages/core/test/percy.test.js index 9768a4d34..e8e4cc21b 100644 --- a/packages/core/test/percy.test.js +++ b/packages/core/test/percy.test.js @@ -30,6 +30,12 @@ describe('Percy', () => { delete process.env.PERCY_CLIENT_ERROR_LOGS; }); + const sharedExpectBlockForSuggestion = (expectedBody) => { + let lastReq = api.requests['/suggestions/from_logs'].length - 1; + expect(api.requests['/suggestions/from_logs'][lastReq].body) + .toEqual(expectedBody); + }; + it('loads config and intializes client with config', () => { expect(percy.client.config).toEqual(percy.config); }); @@ -287,7 +293,17 @@ describe('Percy', () => { it('throws when the port is in use', async () => { await expectAsync(percy.start()).toBeResolved(); await expectAsync(Percy.start({ token: 'PERCY_TOKEN' })) - .toBeRejectedWithError('Percy is already running or the port is in use'); + .toBeRejectedWithError('Percy is already running or the port 5338 is in use'); + + sharedExpectBlockForSuggestion({ + data: { + logs: [ + { + message: 'Percy is already running or the port 5338 is in use' + } + ] + } + }); }); it('queues build creation when uploads are deferred', async () => { @@ -327,6 +343,13 @@ describe('Percy', () => { // processing deferred uploads should not result in a new build await percy.flush(); expect(api.requests['/builds']).toBeUndefined(); + sharedExpectBlockForSuggestion({ + data: { + logs: [ + { message: 'This operation was aborted' } + ] + } + }); }); it('has projectType', async () => { @@ -521,7 +544,16 @@ describe('Percy', () => { await expectAsync(percy.stop()).toBeResolved(); expect(api.requests['/builds/123/finalize']).toBeDefined(); - expect(logger.stderr).toEqual([]); + // This is the condition for no snapshot command was called + expect(logger.stderr).toEqual([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ]); + expect(logger.stdout).toContain( '[percy] Finalized build #1: https://percy.io/test/test/123' ); @@ -1116,4 +1148,169 @@ describe('Percy', () => { ])); }); }); + + describe('#suggestionsForFix', () => { + beforeEach(() => { + percy = new Percy({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 }, + clientInfo: 'client-info', + environmentInfo: 'env-info' + }); + percy.build = { id: 1 }; + }); + + describe('when suggestionResponse.length > 0', () => { + describe('for build level error', () => { + it('should log failureReason, suggestion, and doc links', async () => { + spyOn(percy.client, 'getErrorAnalysis').and.returnValue([{ + suggestion: 'some suggestion', + reason_message: 'some failure reason', + failure_reason: 'some failure title', + reference_doc_link: ['Doc Link 1', 'Doc Link 2'] + }]); + + await expectAsync(percy.suggestionsForFix('some_error')).toBeResolved(); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: some failure title', + '[percy] Failure Reason: some failure reason', + '[percy] Suggestion: some suggestion', + '[percy] Refer to the below Doc Links for the same', + '[percy] * Doc Link 1', + '[percy] * Doc Link 2' + ])); + }); + + describe('when no reference doc links is provided', () => { + it('should log failureReason and suggestion', async () => { + spyOn(percy.client, 'getErrorAnalysis').and.returnValue([{ + suggestion: 'some suggestion', + failure_reason: 'some failure reason', + reference_doc_link: null + }]); + + await expectAsync(percy.suggestionsForFix('some_error')).toBeResolved(); + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: some failure reason', + '[percy] Failure Reason: undefined', + '[percy] Suggestion: some suggestion' + ])); + }); + }); + }); + + describe('for snapshotLevel error', () => { + it('should log failureReason, suggestion, and doc links with snapshotName', async () => { + spyOn(percy.client, 'getErrorAnalysis').and.returnValue([{ + suggestion: 'some suggestion', + failure_reason: 'some failure reason', + reference_doc_link: ['Doc Link 1', 'Doc Link 2'] + }]); + + await expectAsync(percy.suggestionsForFix('some_error', { + snapshotLevel: true, + snapshotName: 'Snapshot 1' + })).toBeResolved(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected erorr for Snapshot: Snapshot 1', + '[percy] Failure: some failure reason', + '[percy] Failure Reason: undefined', + '[percy] Suggestion: some suggestion', + '[percy] Refer to the below Doc Links for the same', + '[percy] * Doc Link 1', + '[percy] * Doc Link 2' + ])); + }); + }); + }); + + describe('when response throw error', () => { + describe('when Request failed with error code of EHOSTUNREACH', () => { + it('should catch and logs expected error', async () => { + spyOn(percy.client, 'getErrorAnalysis').and.rejectWith({ code: 'EHOSTUNREACH', message: 'some error' }); + + await expectAsync(percy.suggestionsForFix('some_error')).toBeResolved(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] percy.io might not be reachable, check network connection, proxy and ensure that percy.io is whitelisted.', + '[percy] If inside a proxied envirnment, please configure the following environment variables: HTTP_PROXY, [ and optionally HTTPS_PROXY if you need it ]. Refer to our documentation for more details', + '[percy] Unable to analyze error logs' + ])); + }); + }); + + describe('when Request failed with error code ECONNREFUSED and HTTPS_PROXY env is enabled', () => { + beforeEach(() => { + process.env.HTTPS_PROXY = 'https://abc.com'; + }); + + afterEach(() => { + delete process.env.HTTPS_PROXY; + }); + + it('should catch and logs expected error', async () => { + spyOn(percy.client, 'getErrorAnalysis').and.rejectWith({ code: 'ECONNREFUSED', message: 'some error' }); + + await expectAsync(percy.suggestionsForFix('some_error')).toBeResolved(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] percy.io might not be reachable, check network connection, proxy and ensure that percy.io is whitelisted.', + '[percy] Unable to analyze error logs' + ])); + }); + }); + + describe('when request failed due to some unexpected issue', () => { + it('should catch and logs expected error', async () => { + spyOn(percy.client, 'getErrorAnalysis').and.rejectWith('some_error'); + + await expectAsync(percy.suggestionsForFix('some_error', { + snapshotLevel: true, + snapshotName: 'Snapshot 1' + })).toBeResolved(); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Unable to analyze error logs' + ])); + }); + }); + }); + }); + + describe('#checkForNoSnapshotCommandError', () => { + it('should log No snapshot command was called', async () => { + percy = new Percy({ + token: 'PERCY_TOKEN', + snapshot: { widths: [1000] }, + discovery: { concurrency: 1 } + }); + + await percy.start(); + await percy.stop(true); + + expect(logger.stderr).toEqual(jasmine.arrayContaining([ + '[percy] Detected error for percy build', + '[percy] Failure: Snapshot command was not called', + '[percy] Failure Reason: Snapshot Command was not called. please check your CI for errors', + '[percy] Suggestion: Try using percy snapshot command to take snapshots', + '[percy] Refer to the below Doc Links for the same', + '[percy] * https://www.browserstack.com/docs/percy/take-percy-snapshots/' + ])); + }); + + it('should not log No snapshot command was called', async () => { + await percy.start(); + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: '' + }); + await percy.stop(true); + expect(logger.stderr).toEqual(jasmine.arrayContaining([])); + }); + }); });