fix: skip caching empty resources (#458)

* fix: skip caching empty resources

skip caching empty resources (browser resource hints)

* ✅ Add cache test for resource hints

* 🔥 Remove redundant live site test

We have a locally controlled test for responsive images, the live sites only
cause pain since we cannot maintain them.

Co-authored-by: Blake Newman <blake.newman@askattest.com>

src/utils/response-cache.ts

@@ -10,6 +10,7 @@ let responseCache = {} as any
  * it so servers aren't being hounded for the same asset over and over again.
  */
 export async function cacheResponse(response: Response, logger: any) {
+  const request = response.request()
   const responseUrl = response.url()
   const statusCode = response.status()
 
@@ -22,6 +23,12 @@ export async function cacheResponse(response: Response, logger: any) {
     return
   }
 
+  if (request.resourceType() === 'other' && (await response.text()).length === 0) {
+    // Skip empty other resource types (browser resource hints)
+    logger.debug(`Skipping caching [is_empty_other]: ${request.url()}`)
+    return
+  }
+
   try {
     const buffer = await response.buffer()
 

test/integration/agent-integration.test.ts

@@ -78,12 +78,6 @@ describe('Integration test', () => {
       expect(domSnapshot).contains('badssl.com')
     })
 
-    it('snapshots a complex website with responsive images', async () => {
-      await page.goto('https://polaris.shopify.com/')
-      const domSnapshot = await snapshot(page, 'Polaris snapshot', {widths: [300, 1200]})
-      expect(domSnapshot).contains('Shopify Polaris')
-    })
-
     it('snapshots a complex website with CSSOM', async () => {
       await page.goto('https://buildkite.com/')
       const domSnapshot = await snapshot(page, 'Buildkite snapshot')

test/unit/utils/response-cache.test.ts

@@ -2,12 +2,19 @@ import { expect } from 'chai'
 import { _setResponseCache, cacheResponse, getResponseCache } from '../../../src/utils/response-cache'
 
 // Mock logger
-const logger = { debug() { return '' }}
+const logger = { debug: () => '' }
 const defaultResponse = {
-  url() { return 'http://example.com/foo.txt' },
-  status() { return 200 },
-  headers() { return 'fake headers' },
-  buffer() { return 'hello' },
+  url: () => 'http://example.com/foo.txt',
+  status: () => 200,
+  headers: () => 'fake headers',
+  buffer: () => 'hello',
+  text() { return this.buffer() },
+  request() {
+    return {
+      resourceType: () => 'other',
+      url: () => this.url(),
+    }
+  },
 } as any
 
 describe('Response cache util', () => {
@@ -26,7 +33,7 @@ describe('Response cache util', () => {
   })
 
   it('201 status code response adds to the cache', async () => {
-    await cacheResponse({ ...defaultResponse, status() { return  201 } }, logger)
+    await cacheResponse({ ...defaultResponse, status: () => 201 }, logger)
 
     expect(getResponseCache('http://example.com/foo.txt')).to.eql({
       status: 201,
@@ -37,7 +44,7 @@ describe('Response cache util', () => {
 
   it('calling the cache with the same URL does nothing', async () => {
     await cacheResponse(defaultResponse, logger)
-    await cacheResponse({ ...defaultResponse, status() { return 201 } }, logger)
+    await cacheResponse({ ...defaultResponse, status: () => 201 }, logger)
 
     expect(getResponseCache('http://example.com/foo.txt')).to.eql({
       status: 200,
@@ -47,10 +54,16 @@ describe('Response cache util', () => {
   })
 
   it('non-200 status code response does not add to the cache', async () => {
-    await cacheResponse({ ...defaultResponse, status() { return 300 } }, logger)
-    await cacheResponse({ ...defaultResponse, status() { return 500 } }, logger)
-    await cacheResponse({ ...defaultResponse, status() { return 401 } }, logger)
-    await cacheResponse({ ...defaultResponse, status() { return 404 } }, logger)
+    await cacheResponse({ ...defaultResponse, status: () => 300 }, logger)
+    await cacheResponse({ ...defaultResponse, status: () => 500 }, logger)
+    await cacheResponse({ ...defaultResponse, status: () => 401 }, logger)
+    await cacheResponse({ ...defaultResponse, status: () => 404 }, logger)
+
+    expect(getResponseCache('http://example.com/foo.txt')).to.eql(undefined)
+  })
+
+  it('does not cache browser hints', async () => {
+    await cacheResponse({ ...defaultResponse, buffer: () => '' }, logger)
 
     expect(getResponseCache('http://example.com/foo.txt')).to.eql(undefined)
   })