fix(gatsby): refresh browser when receiving update and runtime errored #27467

pieh · 2020-10-15T09:20:32Z

Description

This adds crude recovery mechanism: once runtime error was found attach listeners to refresh the browser on either webpack update or data update.

Recently we fixed queueing of query running when editing query text ( #27408 ). However in runtime when users update query and component at the same time, runtime can get in error state because webpack and data updates are not applied at the same time (meaning that we can have either older component or older content for some period of time that can lead to runtime errors). This change allow to recover from situation like that.

--edit:
Please note that the change here is not just for case described above and it's rather generic. Above we ideally fix at some point by synchronising applying updates from webpack HMR and query results from websocket to avoid even getting in runtime errors scenarios, but this does cover more cases than just that ... and with some quirks it does "fix" that problem too (at least users don't have to manually refresh browser tabs)

This also updates react-hot-loader which does change the overlay look (and looks like overlay used by fast-refresh, because it actually is) - this might not be needed for this PR and possibly I can revert that part (I started with bumping overlay thinking I would be able to make use of dismissRuntimeErrors that was exposed in newer version of overlay, but ended up not using it):

---EDIT:
Above screenshots are from "night mode", based on prefers-color-scheme: dark media feature (that react-error-overlay now supports). For users with preference set to light, it will keep looking as it used to.

I added few basic tests for error overlay interactions: (make sure we start with working site -> break something intentionally -> assert error overlay -> fix the problem -> assert that browser automatically recovered)

pieh · 2020-10-15T17:46:34Z

e2e-tests/development-runtime/cypress/integration/hot-reloading/error-handling/compile-error.js

+
+describe(`testing error overlay and ability to automatically recover from webpack compile errors`, () => {
+  it(`displays content initially (no errors yet)`, () => {
+    cy.visit(`/error-handling/compile-error/`).waitForRouteChange()


Important part in those tests that we have just single initial cy.visit and we don't interact with page in any way (don't manually refresh, don't click anything, we only do changes to either source code or content and assert that fixing the problem cause runtime to work again automatically - even if we do refresh browser ourselves, user doesn't have to).

pieh · 2020-10-15T17:49:12Z

packages/gatsby/cache-dir/error-overlay-handler.js

+  let registeredReloadListeners = false
+  function onError() {
+    if (registeredReloadListeners) {
+      return
+    }
+
+    // Inspired by `react-dev-utils` HMR client:
+    // If there was unhandled error, reload browser
+    // on next HMR update
+    module.hot.addStatusHandler(status => {
+      if (status === `apply` || status === `idle`) {
+        window.location.reload()
+      }
+    })
+
+    // Additionally in Gatsby case query result updates can cause
+    // runtime error and also fix them, so reload on data updates
+    // as well
+    ___emitter.on(`pageQueryResult`, () => {
+      window.location.reload()
+    })
+    ___emitter.on(`staticQueryResult`, () => {
+      window.location.reload()
+    })
+
+    registeredReloadListeners = true
+  }
  ReactErrorOverlay.startReportingRuntimeErrors({
-    onError: () => {},
+    onError,
    filename: `/commons.js`,
  })
+
+  // ReactErrorOverlay `onError` handler is triggered pretty late
+  // so we attach same error/unhandledrejection as ReactErrorOverlay
+  // to be able to detect runtime error and setup listeners faster
+  window.addEventListener(`error`, onError)
+  window.addEventListener(`unhandledrejection`, onError)
+


This is actual change (lot of test boilerplate in this PR, so I do want to highlight code change, so it's not lost)

vladar

Looks good to me, thanks! 👍

jablonski · 2020-10-19T07:33:04Z

This change seems to break existing code. With Gatsby 2.24.80 our Dev-Site is constantly reloading, while it works fine with 2.24.79. We have currently some unfixed ESLint-problems (no errors, just warnings) in the browser console, which should not trigger a reload.

But maybe the problem arises from this line:

32:6 warning React Hook useEffect has a missing dependency: '...'. Either include it or remove the dependency array react-hooks/exhaustive-deps
✖ 1 problem (0 errors, 1 warning)
0 errors and 1 warning potentially fixable with the --fix option.

vladar · 2020-10-19T07:59:19Z

Hi @jablonski !

Thanks for reporting. Can you please open a new issue with a reproduction?

jablonski · 2020-10-19T09:36:15Z

I've tracked down the problem... we are including an external consent service script into our Gatsby site which works on 127.0.0.1, but throws an error on localhost. This error triggers the constant reload.

Maybe this new behaviour should be configurable? I can image lots of users who will encounter Javascript errors related to third party scripts which cannot be controlled under all circumstances (think of different environments / states of the application etc.pp.).

pieh · 2020-10-19T09:45:23Z

Maybe this new behaviour should be configurable? I can image lots of users who will encounter Javascript errors related to third party scripts which cannot be controlled under all circumstances (think of different environments / states of the application etc.pp.).

How does this 3rd party script get's added? Is it part of react component tree or completely separate? I could possibly change from all runtime errors to using React error boundaries to catch only react render runtime errors and not react to 3rd party scripts (as long as they are not party of react tree)

jablonski · 2020-10-19T09:48:45Z

Actually this script is loaded via Helmet within our generic page template:

      <script
        type="application/javascript"
        src="https://app.usercentrics.eu/latest/main.js"
        id="..."
      ></script>

pieh · 2020-10-19T09:53:07Z

Do you manually call functions from that script or it automatically runs what it needs to?

jablonski · 2020-10-19T09:58:47Z

The problem arises by just loading the script...

We have some onClick-Handlers which trigger functions from this script later on:

<a onClick={() => typeof window === "object" && window["usercentrics"] && window["usercentrics"].updateConsents([ { "templateId": ucTemplateId, "status": true }, ]) } > Activate Google Maps </a>

pieh · 2020-10-19T12:24:04Z

I did reproduce the problem with just adding external script (via helmet) that just throws.

From my research:
react-error-overlay doesn't show the overlay for this, because https://github.com/facebook/create-react-app/blob/master/packages/react-error-overlay/src/utils/getStackFrames.js#L14-L46 returns null for those and this bail early before showing overlay.

What I can try doing here is to use react error boundary instead of catching all runtime errors - then errors from 3rd party scripts wouldn't trigger . I've seen some weird behaviour with react-hot-loader tho, so it might not work - TBD

…e errored (#27467)" This reverts commit f227e85.

…e errored (#27467)" (#28034) This reverts commit f227e85.

…e errored (#27467)" (#28034) This reverts commit f227e85. (cherry picked from commit 076b59f)

…e errored (#27467)" (#28034) (#28038) This reverts commit f227e85. (cherry picked from commit 076b59f)

#27467) * fix(gatsby): refresh browser when receiving update and runtime errored * add query validation error test * add test cases for changing content causing and fixing runtime errors * adjust test names (cherry picked from commit f227e85)

gatsbot bot added the status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer label Oct 15, 2020

pieh added topic: core Relates to Gatsby's core (e.g. page loading, reporter, state machine) and removed status: triage needed Issue or pull request that need to be triaged and assigned to a reviewer labels Oct 15, 2020

pieh added 2 commits October 15, 2020 15:41

fix(gatsby): refresh browser when receiving update and runtime errored

dacf891

add query validation error test

907b180

pieh force-pushed the retry-rendering-on-updates branch from 3957bea to 907b180 Compare October 15, 2020 13:42

add test cases for changing content causing and fixing runtime errors

042cab1

pieh marked this pull request as ready for review October 15, 2020 17:40

adjust test names

268a122

pieh commented Oct 15, 2020

View reviewed changes

vladar approved these changes Oct 16, 2020

View reviewed changes

pieh merged commit f227e85 into master Oct 16, 2020

delete-merged-branch bot deleted the retry-rendering-on-updates branch October 16, 2020 11:04

pieh added a commit that referenced this pull request Nov 13, 2020

Revert "fix(gatsby): refresh browser when receiving update and runtim…

e7bc866

…e errored (#27467)" This reverts commit f227e85.

gatsbybot pushed a commit that referenced this pull request Nov 13, 2020

Revert "fix(gatsby): refresh browser when receiving update and runtim…

076b59f

…e errored (#27467)" (#28034) This reverts commit f227e85.

vladar pushed a commit that referenced this pull request Nov 13, 2020

Revert "fix(gatsby): refresh browser when receiving update and runtim…

be489fa

…e errored (#27467)" (#28034) This reverts commit f227e85. (cherry picked from commit 076b59f)

vladar added a commit that referenced this pull request Nov 13, 2020

Revert "fix(gatsby): refresh browser when receiving update and runtim…

9f24f25

…e errored (#27467)" (#28034) (#28038) This reverts commit f227e85. (cherry picked from commit 076b59f)

LekoArts mentioned this pull request Mar 15, 2021

chore: Add Fast Refresh/Hot Reloading E2E tests #30162

Merged

This was referenced Apr 15, 2024

[Snyk] Security upgrade gatsby from 2.18.12 to 2.24.80 nidhi42/gatsby#3412

Open

[Snyk] Security upgrade gatsby from 2.18.12 to 2.24.80 nidhi42/gatsby#3422

Open

monizb mentioned this pull request Apr 15, 2024

[Snyk] Security upgrade gatsby from 2.18.12 to 2.24.80 monizb/gatsby#1442

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(gatsby): refresh browser when receiving update and runtime errored #27467

fix(gatsby): refresh browser when receiving update and runtime errored #27467

pieh commented Oct 15, 2020 •

edited

Loading

pieh Oct 15, 2020

pieh Oct 15, 2020 •

edited

Loading

vladar left a comment

jablonski commented Oct 19, 2020

vladar commented Oct 19, 2020

jablonski commented Oct 19, 2020

pieh commented Oct 19, 2020

jablonski commented Oct 19, 2020

pieh commented Oct 19, 2020

jablonski commented Oct 19, 2020

pieh commented Oct 19, 2020

fix(gatsby): refresh browser when receiving update and runtime errored #27467

fix(gatsby): refresh browser when receiving update and runtime errored #27467

Conversation

pieh commented Oct 15, 2020 • edited Loading

Description

pieh Oct 15, 2020

Choose a reason for hiding this comment

pieh Oct 15, 2020 • edited Loading

Choose a reason for hiding this comment

vladar left a comment

Choose a reason for hiding this comment

jablonski commented Oct 19, 2020

vladar commented Oct 19, 2020

jablonski commented Oct 19, 2020

pieh commented Oct 19, 2020

jablonski commented Oct 19, 2020

pieh commented Oct 19, 2020

jablonski commented Oct 19, 2020

pieh commented Oct 19, 2020

pieh commented Oct 15, 2020 •

edited

Loading

pieh Oct 15, 2020 •

edited

Loading