diff --git a/packages/core/src/network.js b/packages/core/src/network.js index 9cf59ce86..437f9f9bc 100644 --- a/packages/core/src/network.js +++ b/packages/core/src/network.js @@ -324,6 +324,44 @@ export class Network { /* istanbul ignore if: race condition paranoia */ if (!request) return; + // Log instrumentation for failed requests + let response = request.response; + let category, reason, details; + + if (response) { + // Request has a response but failed (e.g., 404, 5xx) + if (response.status >= 500 && response.status < 600) { + category = 'asset_load_5xx'; + reason = 'server_error'; + } else if (!ALLOWED_STATUSES.includes(response.status)) { + category = 'asset_not_uploaded'; + reason = 'disallowed_status'; + } + + if (category) { + details = { + url: request.url, + statusCode: response.status, + snapshot: this.meta.snapshot, + requestType: request.type + }; + } + // Failed request without a response (network error) + } else if (event.errorText && event.errorText !== 'net::ERR_FAILED') { + category = 'asset_load_missing'; + reason = 'network_error'; + details = { + url: request.url, + errorText: event.errorText, + snapshot: this.meta.snapshot, + requestType: request.type + }; + } + + if (category) { + logAssetInstrumentation(this.log, category, reason, details); + } + // If request was aborted, keep track of it as we need to cancel any in process callbacks for // such a request to avoid Invalid InterceptionId errors // Note: 404s also show up under ERR_ABORTED and not ERR_FAILED @@ -353,6 +391,38 @@ export class Network { } } +// Logs asset instrumentation for failed/skipped asset loading +function logAssetInstrumentation(log, category, reason, details) { + const categoryMap = { + asset_load_5xx: '[ASSET_LOAD_5XX]', + asset_not_uploaded: '[ASSET_NOT_UPLOADED]', + asset_load_missing: '[ASSET_LOAD_MISSING]' + }; + + const messageMap = { + server_error: 'Server error response', + disallowed_status: 'Disallowed status code', + network_error: 'Network error', + disallowed_hostname: 'Disallowed hostname', + resource_too_large: 'Resource too large', + no_response: 'No response received', + empty_response: 'Empty response', + disallowed_resource_type: 'Disallowed resource type' + }; + + const prefix = categoryMap[category]; + const message = messageMap[reason] || reason; + + log.debug( + `${prefix} ${message}`, + { + ...details, + reason, + instrumentationCategory: category + } + ); +} + // Returns the normalized origin URL of a request function originURL(request) { return normalizeURL((request.redirectChain[0] || request).url); @@ -372,6 +442,11 @@ async function sendResponseResource(network, request, session) { network.log.debug(`Handling request: ${url}`, meta); if (!resource?.root && hostnameMatches(disallowedHostnames, url)) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'disallowed_hostname', { + url, + hostname: new URL(url).hostname, + snapshot: meta.snapshot + }); log.debug('- Skipping disallowed hostname', meta); await send('Fetch.failRequest', { @@ -474,6 +549,11 @@ async function saveResponseResource(network, request, session) { let contentLength = response.headers?.[Object.keys(response.headers).find(key => key.toLowerCase() === 'content-length')]; contentLength = parseInt(contentLength); if (contentLength > MAX_RESOURCE_SIZE) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'resource_too_large', { + url, + size: contentLength, + snapshot: meta.snapshot + }); return log.debug('- Skipping resource larger than 25MB', meta); } let resource = network.intercept.getResource(url); @@ -488,17 +568,54 @@ async function saveResponseResource(network, request, session) { // Don't rename the below log line as it is used in getting network logs in api /* istanbul ignore if: first check is a sanity check */ if (!response) { + logAssetInstrumentation(log, 'asset_load_missing', 'no_response', { + url, + snapshot: meta.snapshot, + requestType: request.type + }); return log.debug('- Skipping no response', meta); } else if (!shouldCapture) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'disallowed_hostname', { + url, + hostname: new URL(url).hostname, + snapshot: meta.snapshot + }); return log.debug('- Skipping remote resource', meta); } else if (!body.length) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'empty_response', { + url, + snapshot: meta.snapshot + }); return log.debug('- Skipping empty response', meta); } else if (body.length > MAX_RESOURCE_SIZE) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'resource_too_large', { + url, + size: body.length, + snapshot: meta.snapshot + }); log.debug('- Missing headers for the requested resource.', meta); return log.debug('- Skipping resource larger than 25MB', meta); } else if (!ALLOWED_STATUSES.includes(response.status)) { + /* istanbul ignore next: ternary branches tested separately */ + const category = (response.status >= 500 && response.status < 600) + ? 'asset_load_5xx' + : 'asset_not_uploaded'; + const reason = (response.status >= 500 && response.status < 600) + ? 'server_error' + : 'disallowed_status'; + logAssetInstrumentation(log, category, reason, { + url, + statusCode: response.status, + snapshot: meta.snapshot, + requestType: request.type + }); return log.debug(`- Skipping disallowed status [${response.status}]`, meta); } else if (!enableJavaScript && !ALLOWED_RESOURCES.includes(request.type)) { + logAssetInstrumentation(log, 'asset_not_uploaded', 'disallowed_resource_type', { + url, + resourceType: request.type, + snapshot: meta.snapshot + }); return log.debug(`- Skipping disallowed resource type [${request.type}]`, meta); } diff --git a/packages/core/test/discovery.test.js b/packages/core/test/discovery.test.js index 16e8e6475..8df646db9 100644 --- a/packages/core/test/discovery.test.js +++ b/packages/core/test/discovery.test.js @@ -783,6 +783,120 @@ describe('Discovery', () => { ); }); + describe('asset instrumentation', () => { + it('logs instrumentation for 5xx errors', async () => { + server.reply('/error.css', () => [502, 'text/plain', 'Bad Gateway']); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM.replace('style.css', 'error.css'), + discovery: { disableCache: true } + }); + + await percy.idle(); + + const logs = logger.instance.query(log => log.debug === 'core:discovery'); + expect(logs.length).toBeGreaterThan(0, 'No core:discovery logs found'); + + const errorLogs = logs.filter(l => l.meta && l.meta.instrumentationCategory === 'asset_load_5xx'); + expect(errorLogs.length).toBeGreaterThan(0, 'No asset_load_5xx logs found'); + expect(errorLogs[0].meta.statusCode).toBe(502); + expect(errorLogs[0].meta.reason).toBe('server_error'); + expect(errorLogs[0].message).toContain('[ASSET_LOAD_5XX]'); + }); + + it('logs instrumentation for resources too large', async () => { + server.reply('/huge.css', () => [200, 'text/css', 'A'.repeat(30_000_000)]); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM.replace('style.css', 'huge.css') + }); + + await percy.idle(); + + const logs = logger.instance.query(log => log.debug === 'core:discovery'); + expect(logs.length).toBeGreaterThan(0, 'No core:discovery logs found'); + + const notUploadedLogs = logs.filter(l => l.meta && l.meta.instrumentationCategory === 'asset_not_uploaded'); + const largeLogs = notUploadedLogs.filter(l => l.meta.reason === 'resource_too_large'); + expect(largeLogs.length).toBeGreaterThan(0, 'No resource_too_large logs found'); + expect(largeLogs[0].meta.size).toBeGreaterThan(25000000); + expect(largeLogs[0].message).toContain('[ASSET_NOT_UPLOADED]'); + }); + + it('logs instrumentation for disallowed status codes', async () => { + server.reply('/notfound.css', () => [404, 'text/plain', 'Not Found']); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM.replace('style.css', 'notfound.css'), + discovery: { disableCache: true } + }); + + await percy.idle(); + + const logs = logger.instance.query(log => log.debug === 'core:discovery'); + expect(logs.length).toBeGreaterThan(0, 'No core:discovery logs found'); + + const notUploadedLogs = logs.filter(l => l.meta && l.meta.instrumentationCategory === 'asset_not_uploaded'); + const disallowedLogs = notUploadedLogs.filter(l => l.meta.reason === 'disallowed_status'); + expect(disallowedLogs.length).toBeGreaterThan(0, 'No disallowed_status logs found'); + expect(disallowedLogs[0].meta.statusCode).toBe(404); + expect(disallowedLogs[0].message).toContain('[ASSET_NOT_UPLOADED]'); + }); + + it('logs instrumentation for empty responses', async () => { + server.reply('/empty.css', () => [200, 'text/css', '']); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM.replace('style.css', 'empty.css'), + discovery: { disableCache: true } + }); + + await percy.idle(); + + const logs = logger.instance.query(log => log.debug === 'core:discovery'); + expect(logs.length).toBeGreaterThan(0, 'No core:discovery logs found'); + + const notUploadedLogs = logs.filter(l => l.meta && l.meta.instrumentationCategory === 'asset_not_uploaded'); + const emptyLogs = notUploadedLogs.filter(l => l.meta && l.meta.reason === 'empty_response'); + expect(emptyLogs.length).toBeGreaterThan(0, 'No empty_response logs found'); + expect(emptyLogs[0].message).toContain('[ASSET_NOT_UPLOADED]'); + }); + + it('logs instrumentation for network errors', async () => { + // Simulate a network error by closing connection without response + server.reply('/aborted.css', req => { + req.socket.destroy(); + return null; + }); + + await percy.snapshot({ + name: 'test snapshot', + url: 'http://localhost:8000', + domSnapshot: testDOM.replace('style.css', 'aborted.css'), + discovery: { disableCache: true } + }); + + await percy.idle(); + + const logs = logger.instance.query(log => log.debug === 'core:discovery'); + expect(logs.length).toBeGreaterThan(0, 'No core:discovery logs found'); + + const missingLogs = logs.filter(l => l.meta && l.meta.instrumentationCategory === 'asset_load_missing'); + expect(missingLogs.length).toBeGreaterThan(0, 'No asset_load_missing logs found'); + expect(missingLogs[0].meta.reason).toBe('network_error'); + expect(missingLogs[0].meta.errorText).toBeDefined(); + expect(missingLogs[0].message).toContain('[ASSET_LOAD_MISSING]'); + }); + }); + it('does not capture duplicate root resources', async () => { let reDOM = dedent`