feat(gatsby): improve error messages at runtime (#29970)

gatsbyjs · Mar 5, 2021 · d37f275 · d37f275
1 parent 5636389
commit d37f275
Show file tree

Hide file tree

Showing 6 changed files with 105 additions and 70 deletions.
diff --git a/e2e-tests/production-runtime/cypress/integration/resource-loading-resilience.js b/e2e-tests/production-runtime/cypress/integration/resource-loading-resilience.js
@@ -1,29 +1,7 @@
-Cypress.on(`uncaught:exception`, (err, runnable) => {
-  // returning false here prevents Cypress from
-  // failing the test
-  console.log(err)
-  return false
-})
-
 const waitForAPIOptions = {
   timeout: 10000,
 }
 
-function assertOnNavigate(
-  page = null,
-  locationAssert,
-  shouldLocationAssert,
-  assertShouldBe
-) {
-  if (page) {
-    cy.getTestElement(page).click()
-  }
-  cy.waitForAPIorTimeout(`onRouteUpdate`, waitForAPIOptions)
-    .location(`pathname`)
-    .should(locationAssert, shouldLocationAssert)
-  cy.getTestElement(`dom-marker`).contains(assertShouldBe)
-}
-
 function runTests(testNameSuffix) {
   it(`Loads index - ${testNameSuffix}`, () => {
     cy.visit(`/`).waitForAPIorTimeout(`onRouteUpdate`, waitForAPIOptions)
@@ -63,11 +41,11 @@ function runTests(testNameSuffix) {
   })
 }
 
-const runBlockedScenario = args => {
+const runBlockedScenario = ({ filter, pagePath }) => {
   beforeEach(() => {
     cy.task("getAssetsForPage", {
-      pagePath: args.pagePath,
-      filter: args.filter,
+      pagePath,
+      filter,
     }).then(urls => {
       for (const url of urls) {
         cy.intercept(url, {
@@ -82,17 +60,14 @@ const runBlockedScenario = args => {
   afterEach(() => {
     // check if assets are actually stubbed
     cy.task("getAssetsForPage", {
-      pagePath: args.pagePath,
-      filter: args.filter,
+      pagePath,
+      filter,
     }).then(urls => {
       expect(Object.keys(cy.state("routes")).length).to.equal(urls.length)
     })
   })
 
-  runTests(`Blocked "${args.filter}" for "${args.pagePath}"`, {
-    ...args,
-    task: "getAssetsForPage",
-  })
+  runTests(`Blocked "${filter}" for "${pagePath}"`)
 }
 
 const runSuiteForPage = (label, pagePath) => {
@@ -103,6 +78,12 @@ const runSuiteForPage = (label, pagePath) => {
         filter: `page-data`,
       })
     })
+    describe(`Missing "${label}" static query results`, () => {
+      runBlockedScenario({
+        pagePath,
+        filter: `static-query-data`,
+      })
+    })
     describe(`Missing "${label}" page page-template asset`, () => {
       runBlockedScenario({
         pagePath,

diff --git a/e2e-tests/production-runtime/cypress/plugins/block-resources.js b/e2e-tests/production-runtime/cypress/plugins/block-resources.js
@@ -17,7 +17,7 @@ const filterAssets = (assetsForPath, filter) => {
   return assetsForPath.filter(asset => {
     if (filter === `all`) {
       return true
-    } else if (filter === `page-data`) {
+    } else if (filter === `page-data` || filter === "static-query-data") {
       return false
     }
 
@@ -55,6 +55,10 @@ function getAssetsForPage({ pagePath, filter }) {
     assets.push(`/${pageDataUrl}`)
   }
 
+  if (filter === `all` || filter === `static-query-data`) {
+    assets.push(`/page-data/sq/d/${pageData.staticQueryHashes[0]}.json`)
+  }
+
   return assets
 }
 

diff --git a/packages/gatsby/cache-dir/__tests__/dev-loader.js b/packages/gatsby/cache-dir/__tests__/dev-loader.js
@@ -376,7 +376,7 @@ describe(`Dev loader`, () => {
 
     it(`should return an error when component cannot be loaded`, async () => {
       const asyncRequires = createAsyncRequires({
-        chunk: false,
+        chunk: () => Promise.resolve(false),
       })
       const devLoader = new DevLoader(asyncRequires, [])
       const pageData = {

diff --git a/packages/gatsby/cache-dir/dev-loader.js b/packages/gatsby/cache-dir/dev-loader.js
@@ -25,13 +25,20 @@ function mergePageEntry(cachedPage, newPageData) {
 
 class DevLoader extends BaseLoader {
   constructor(asyncRequires, matchPaths) {
-    const loadComponent = chunkName =>
-      asyncRequires.components[chunkName]
-        ? asyncRequires.components[chunkName]()
-            .then(preferDefault)
-            // loader will handle the case when component is null
-            .catch(() => null)
-        : Promise.resolve()
+    const loadComponent = chunkName => {
+      if (!asyncRequires.components[chunkName]) {
+        throw new Error(
+          `We couldn't find the correct component chunk with the name "${chunkName}"`
+        )
+      }
+
+      return (
+        asyncRequires.components[chunkName]()
+          .then(preferDefault)
+          // loader will handle the case when component is error
+          .catch(err => err)
+      )
+    }
     super(loadComponent, matchPaths)
 
     const socket = getSocket()

diff --git a/packages/gatsby/cache-dir/loader.js b/packages/gatsby/cache-dir/loader.js
@@ -213,6 +213,13 @@ export class BaseLoader {
     if (this.pageDb.has(pagePath)) {
       const page = this.pageDb.get(pagePath)
       if (process.env.BUILD_STAGE !== `develop` || !page.payload.stale) {
+        if (page.error) {
+          return {
+            error: page.error,
+            status: page.status,
+          }
+        }
+
         return Promise.resolve(page.payload)
       }
     }
@@ -241,8 +248,9 @@ export class BaseLoader {
         component => {
           finalResult.createdAt = new Date()
           let pageResources
-          if (!component) {
+          if (!component || component instanceof Error) {
             finalResult.status = PageResourceStatus.Error
+            finalResult.error = component
           } else {
             finalResult.status = PageResourceStatus.Success
             if (result.notFound === true) {
@@ -270,10 +278,16 @@ export class BaseLoader {
 
           return this.memoizedGet(
             `${__PATH_PREFIX__}/page-data/sq/d/${staticQueryHash}.json`
-          ).then(req => {
-            const jsonPayload = JSON.parse(req.responseText)
-            return { staticQueryHash, jsonPayload }
-          })
+          )
+            .then(req => {
+              const jsonPayload = JSON.parse(req.responseText)
+              return { staticQueryHash, jsonPayload }
+            })
+            .catch(() => {
+              throw new Error(
+                `We couldn't load "${__PATH_PREFIX__}/page-data/sq/d/${staticQueryHash}.json"`
+              )
+            })
         })
       ).then(staticQueryResults => {
         const staticQueryResultsMap = {}
@@ -286,27 +300,42 @@ export class BaseLoader {
         return staticQueryResultsMap
       })
 
-      return Promise.all([componentChunkPromise, staticQueryBatchPromise]).then(
-        ([pageResources, staticQueryResults]) => {
-          let payload
-          if (pageResources) {
-            payload = { ...pageResources, staticQueryResults }
-            finalResult.payload = payload
-            emitter.emit(`onPostLoadPageResources`, {
-              page: payload,
-              pageResources: payload,
-            })
-          }
+      return (
+        Promise.all([componentChunkPromise, staticQueryBatchPromise])
+          .then(([pageResources, staticQueryResults]) => {
+            let payload
+            if (pageResources) {
+              payload = { ...pageResources, staticQueryResults }
+              finalResult.payload = payload
+              emitter.emit(`onPostLoadPageResources`, {
+                page: payload,
+                pageResources: payload,
+              })
+            }
 
-          this.pageDb.set(pagePath, finalResult)
+            this.pageDb.set(pagePath, finalResult)
 
-          return payload
-        }
+            if (finalResult.error) {
+              return {
+                error: finalResult.error,
+                status: finalResult.status,
+              }
+            }
+
+            return payload
+          })
+          // when static-query fail to load we throw a better error
+          .catch(err => {
+            return {
+              error: err,
+              status: PageResourceStatus.Error,
+            }
+          })
       )
     })
 
     inFlightPromise
-      .then(response => {
+      .then(() => {
         this.inFlightDb.delete(pagePath)
       })
       .catch(error => {
@@ -449,13 +478,20 @@ const createComponentUrls = componentChunkName =>
 
 export class ProdLoader extends BaseLoader {
   constructor(asyncRequires, matchPaths) {
-    const loadComponent = chunkName =>
-      asyncRequires.components[chunkName]
-        ? asyncRequires.components[chunkName]()
-            .then(preferDefault)
-            // loader will handle the case when component is null
-            .catch(() => null)
-        : Promise.resolve()
+    const loadComponent = chunkName => {
+      if (!asyncRequires.components[chunkName]) {
+        throw new Error(
+          `We couldn't find the correct component chunk with the name ${chunkName}`
+        )
+      }
+
+      return (
+        asyncRequires.components[chunkName]()
+          .then(preferDefault)
+          // loader will handle the case when component is error
+          .catch(err => err)
+      )
+    }
 
     super(loadComponent, matchPaths)
   }

diff --git a/packages/gatsby/cache-dir/production-app.js b/packages/gatsby/cache-dir/production-app.js
@@ -152,9 +152,16 @@ apiRunnerAsync(`onClientEntry`).then(() => {
 
   publicLoader.loadPage(browserLoc.pathname).then(page => {
     if (!page || page.status === PageResourceStatus.Error) {
-      throw new Error(
-        `page resources for ${browserLoc.pathname} not found. Not rendering React`
-      )
+      const message = `page resources for ${browserLoc.pathname} not found. Not rendering React`
+
+      // if the chunk throws an error we want to capture the real error
+      // This should help with https://github.com/gatsbyjs/gatsby/issues/19618
+      if (page && page.error) {
+        console.error(message)
+        throw page.error
+      }
+
+      throw new Error(message)
     }
 
     window.___webpackCompilationHash = page.page.webpackCompilationHash