return no-cache on 404 (#2609)

* return no-cache on 404 * rename function, remove redundant no-cache * feedback * Should close #2572 - french classes heisenbug
okTurtles · Feb 12, 2025 · b35c12b · b35c12b
1 parent a1669f5
commit b35c12b
Show file tree

Hide file tree

Showing 2 changed files with 14 additions and 23 deletions.
diff --git a/backend/routes.js b/backend/routes.js
@@ -71,6 +71,12 @@ const route = new Proxy({}, {
   }
 })
 
+// helper function that returns 404 and prevents client from caching the 404 response
+// which can sometimes break things: https://github.com/okTurtles/group-income/issues/2608
+function notFoundNoCache (h) {
+  return h.response().code(404).header('Cache-Control', 'no-store')
+}
+
 // RESTful API routes
 
 // TODO: Update this regex once `chel` uses prefixed manifests
@@ -277,7 +283,7 @@ route.GET('/latestHEADinfo/{contractID}', {
     const HEADinfo = await sbp('chelonia/db/latestHEADinfo', contractID)
     if (!HEADinfo) {
       console.warn(`[backend] latestHEADinfo not found for ${contractID}`)
-      return Boom.notFound()
+      return notFoundNoCache(h)
     }
     return HEADinfo
   } catch (err) {
@@ -458,13 +464,7 @@ route.POST('/file', {
 
 // Serve data from Chelonia DB.
 // Note that a `Last-Modified` header isn't included in the response.
-route.GET('/file/{hash}', {
-  cache: {
-    // Do not set other cache options here, to make sure the 'otherwise' option
-    // will be used so that the 'immutable' directive gets included.
-    otherwise: 'public,max-age=31536000,immutable'
-  }
-}, async function (request, h) {
+route.GET('/file/{hash}', {}, async function (request, h) {
   const { hash } = request.params
 
   if (hash.startsWith('_private')) {
@@ -473,9 +473,10 @@ route.GET('/file/{hash}', {
 
   const blobOrString = await sbp('chelonia/db/get', `any:${hash}`)
   if (!blobOrString) {
-    return Boom.notFound()
+    return notFoundNoCache(h)
   }
-  return h.response(blobOrString).etag(hash)
+  return h.response(blobOrString).code(200).etag(hash)
+    .header('Cache-Control', 'public,max-age=31536000,immutable')
 })
 
 route.POST('/deleteFile/{hash}', {
@@ -675,7 +676,7 @@ route.GET('/kv/{contractID}/{key}', {
 
   const result = await sbp('chelonia/db/get', `_private_kv_${contractID}_${key}`)
   if (!result) {
-    return Boom.notFound()
+    return notFoundNoCache(h)
   }
 
   return h.response(result).etag(createCID(result))
@@ -804,7 +805,7 @@ route.GET('/zkpp/{name}/auth_hash', {
   try {
     const challenge = await getChallenge(req.params['name'], req.query['b'])
 
-    return challenge || Boom.notFound()
+    return challenge || notFoundNoCache(h)
   } catch (e) {
     e.ip = req.headers['x-real-ip'] || req.info.remoteAddress
     console.error(e, 'Error at GET /zkpp/{name}/auth_hash: ' + e.message)

diff --git a/test/cypress/integration/group-contributions.spec.js b/test/cypress/integration/group-contributions.spec.js
@@ -26,15 +26,6 @@ function addNonMonetaryContribution (name) {
   })
 }
 
-function assertNonMonetaryEditableValue (name) {
-  // Need to wait until the event is processed
-  cy.giEmptyInvocationQueue()
-
-  cy.getByDT('buttonEditNonMonetaryContribution').click()
-  cy.getByDT('inputNonMonetaryContribution').should('have.value', name)
-  cy.getByDT('buttonCancelNonMonetaryContribution').click()
-}
-
 function assertGraphicSummary (legendListItems) {
   cy.getByDT('groupPledgeSummary', 'ul').within(([list]) => {
     legendListItems.forEach((legendText, index) => {
@@ -373,8 +364,7 @@ describe('Contributions', () => {
     cy.getByDT('buttonEditNonMonetaryContribution').click()
     cy.getByDT('inputNonMonetaryContribution').clear()
     cy.getByDT('inputNonMonetaryContribution').type('French classes{enter}')
-    assertNonMonetaryEditableValue('French classes')
-
+    cy.giEmptyInvocationQueue() // wait for edits to go through
     cy.getByDT('givingList', 'ul')
       .get('li.is-editable')
       .should('have.length', 1)