improve test debugging and loading of cached redirects json file (git…

…hub#23584)

* improve test debugging and loading of cached redirects json file

* exception for testing 500 page itself

lib/redirects/precompile.js

@@ -27,7 +27,9 @@ function diskMemoize(filePath, asyncFn, maxAgeSeconds = 60 * 60) {
       }
       log(`Redirects disk-cache ${filePath} too old`)
     } catch (err) {
-      if (err.code !== 'ENOENT') throw err
+      if (err instanceof SyntaxError) {
+        console.warn(`Syntax error when trying to JSON parse the ${filePath}`, err)
+      } else if (err.code !== 'ENOENT') throw err
     }
     log(`Redirects disk-cache MISS on ${filePath}`)
     const promise = asyncFn(...args)

middleware/handle-errors.js

@@ -27,6 +27,16 @@ async function logException(error, req) {
 }
 
 export default async function handleError(error, req, res, next) {
+  // When you run tests that use things doing get() requests in
+  // our supertest handler, if something goes wrong anywhere in the app
+  // and its middlewares, you get a 500 but the error is never displayed
+  // anywhere. So this is why we log it additionally.
+  // Note, not using console.error() because it's arguably handled.
+  // Some tests might actually expect a 500 error.
+  if (process.env.NODE_ENV === 'test') {
+    console.warn('An error occurrred in some middleware handler', error)
+  }
+
   try {
     // If the headers have already been sent or the request was aborted...
     if (res.headersSent || req.aborted) {

tests/helpers/supertest.js

@@ -34,8 +34,11 @@ export const head = (helpers.head = async function (route, opts = { followRedire
 
 export const post = (helpers.post = (route) => request('post', route))
 
-export const getDOM = (helpers.getDOM = async function (route, headers) {
+export const getDOM = (helpers.getDOM = async function (route, headers, allow500s = false) {
   const res = await helpers.get(route, { followRedirects: true, headers })
+  if (!allow500s && res.status >= 500) {
+    throw new Error(`Server error (${res.status}) on ${route}`)
+  }
   const $ = cheerio.load(res.text || '', { xmlMode: true })
   $.res = Object.assign({}, res)
   return $
@@ -45,5 +48,8 @@ export const getDOM = (helpers.getDOM = async function (route, headers) {
 // e.g. await getJSON('/en?json=breadcrumbs')
 export const getJSON = (helpers.getJSON = async function (route) {
   const res = await helpers.get(route, { followRedirects: true })
+  if (res.status >= 500) {
+    throw new Error(`Server error (${res.status}) on ${route}`)
+  }
   return JSON.parse(res.text)
 })

tests/rendering/server.js

@@ -183,7 +183,7 @@ describe('server', () => {
   })
 
   test('renders a 500 page when errors are thrown', async () => {
-    const $ = await getDOM('/_500')
+    const $ = await getDOM('/_500', undefined, true)
     expect($('h1').text()).toBe('Ooops!')
     expect($.text().includes('It looks like something went wrong.')).toBe(true)
     expect(