Network errors not caught by error boundary when using `useLazyLoadQuery`, `useFragment` and a `store-and/or-network` fetch policy in non-concurrent mode (and React Native)

[levibuzolic]

Note: I originally opened this issue as being React Native specific, however after testing in React DOM v17, this appears to be an issue common to all version of React running in non-concurrent mode (ie. ReactDOM.render) and all versions of React Native (which doesn't support concurrent mode prior to the new architecture).

This issue happens when using useLazyLoadQuery with a fetch policy of store-and-network or store-or-network and the data requiring network is specified in a useFragment. If a network error is raised, rather than being caught by the closest error boundary the component will instead render to completion without it's required data, violating the type safety contracts. In production environments this results in JS level errors as the rendered components try and access data that isn't present.

I've created a minimal reproduction in a CodeSandbox where the network layer intentionally throws an Error to simulate the behaviour of a failing fetch/error response.

React: 18.2.0 Relay: 14.1.0 Concurrent Mode: disabled

Expected behaviour

The network error should halt the component render, and the network error should bubble up to be caught by the closest error boundary component.

Observed behaviour

The network error is ignored and the component renders to completion. This results in JS TypeErrors as the component will try and access data that is expected to be present. This breaks the type safety contract.

Curiously if you trigger another render in the component (e.g. by setting state) the error will then be correctly handled, it's only on the initial render that the error is ignored.

Other test cases

❌  = bug is present
✅  = correct behaviour

❌ React 18, Relay 14.1 - concurrent mode disabled

React: 18.2.0 Relay: 14.1.0 Concurrent Mode: disabled

❌ React 17, Relay 14.1

React: 17.0.2 Relay: 14.1.0

React 17 doesn't support concurrent mode, so it exhibits the same issue.

❌ React 18.2, Relay 11 - concurrent mode disabled

React: 18.2.0 Relay: 11.0.2 Concurrent Mode: disabled

This issue appears to be present in all versions of Relay hooks (>=11.0.0)

❌ React 18, Relay nightly 0.0.0-main-f8ccd9af - concurrent mode disabled

React: 18.2.0 Relay: 0.0.0-main-f8ccd9af f8ccd9af Concurrent Mode: disabled

Issue is present in the latest published nightly (as of December 23rd, 2022)

✅ React 18 - Concurrent mode enabled

React: 18.2.0 Relay: 14.1.0 Concurrent Mode: enabled

With concurrent mode enabled, the network error is successfully caught by the parent error boundary component.

✅ React 18 - without fragment - concurrent mode disabled

React: 18.2.0 Relay: 14.1.0 Concurrent Mode: disabled

If we omit the useFragment and instead define the data requirements entirely within the useLazyLoadQuery, the network error is handled correctly.

✅ fetchPolicy: 'network-only' - concurrent mode disabled

React: 18.2.0 Relay: 14.1.0 Concurrent Mode: disabled

When using fetchPolicy: 'network-only' the error is handled correctly.

[levibuzolic]

Edited: This test case is another example of the issue when using React Native Web, however it's probably not as useful as the reproduction in the main issue.

Original comment with another minimal reproduction

I've created a simpler reproduction in a branch without the noise of the other test cases and deployed the React Native Web version to Vercel.

https://react-native-relay-network-error.vercel.app/

Tapping on the Load button will mount the <Query> component.

Expected behaviour: Error boundary catches the simulated network error.

Observed behaviour: Error boundary catches a JS error from trying to access query.product.name

If anybody has any suggestions/ideas, please let me know, but I'll continue looking through the Relay code to see if I can work out why this case doesn't behave correctly.

[alunyov]

Thank you @levibuzolic for detailed bug report! I'm looking into this

[alunyov]

So, I don't think I have a solution, or even a real root cause. But I've created a unit-test based on your examples: https://github.com/facebook/relay/pull/4157

I think this is happening in the case when there is no concurrent features enabled, and Relay's renderPolicy = 'partial'.

So, theoretically, you can workaround this issue in by setting the renderPolicy: full (I think this can even be set on the environment level: https://github.com/facebook/relay/blob/83ae9735156816ad20d655114db4354fc4029459/packages/relay-runtime/store/RelayModernEnvironment.js#L80

[levibuzolic]

@alunyov awesome, thanks for setting up the test case, I should have done that from the beginning!

Looks like UNSTABLE_defaultRenderPolicy: 'full' on the environment doesn't work, which seems unusual, however setting UNSTABLE_renderPolicy: 'full' on the useLazyLoadQuery directly does work. That's more than a good enough work around for us for now.

I wonder if it'd almost make sense to set the policy to full by default for non-concurrent environments (at least until we can work out the actual root cause)? 🤔

Anyway, thanks a lot for looking into this, now that there's a test case I'll take another look and try and see if I can find the root cause.

[alloy]

@alunyov For my understanding, in concurrent mode this would throw the error from the useFragment call-sites? And do you have any idea why this might be happening?

So, theoretically, you can workaround this issue in by setting the renderPolicy: full (I think this can even be set on the environment level

This works indeed, but has the expected downside of now suspending the tree from the use*Query call-site.

[avoc-ado]

UNSTABLE_defaultRenderPolicy: 'full' on the relay hook fixes this for me. Levi also reported the New Architecture also fixes it.

I've found the bug occurs for me when an error is thrown in the relay fetchFn after a promise is awaited. I tested react-relay 16 and 17

This relay syntax fragment Foo on Query @refetchable can reliably cause the bug. To reproduce: 1) use this syntax 2) disconnect the internet (or throw a simulated error after an awaited promise in fetchFn) 3) fetch the query (e.g. by navigating) and force the error. You will observe data.Foo is undefined. The expected behaviour is the network error is rethrown at the relay hook

query searchScreenQuery($query: String!, $tokenSortBy: String) {
  ...ProfilesListFragment @arguments(query: $query)
}

const fragment = graphql`
  fragment ProfilesListFragment on Query
  @argumentDefinitions(
    query: { type: "String" }
    sortBy: { type: "String" }
    cursor: { type: "String" }
    count: { type: "Int", defaultValue: 25 }
  )
  @refetchable(queryName: "ProfilesListRefetchQuery") {
    searchProfiles(query: $query, sortBy: $sortBy, after: $cursor, first: $count)
      @connection(key: "ProfilesListFragment_searchProfiles") {
      edges { }
    }
}

however this more vanilla syntax behaves perfectly on error. The error is rethrown at the relay hook every time

const query = graphql`
  query searchScreenQuery($query: String!, $sortBy: String, $cursor: String, $count: Int) {
    searchProfiles(query: $query, sortBy: $sortBy, after: $cursor, first: $count)
      @connection(key: "ProfilesListFragment_searchProfiles") {
      edges {
        node {
          ...ProfilesListFragment
        }
      }
    }
  }
`;
export const ProfileListFragment = graphql`
  fragment ProfilesListFragment on Profile {
    id
    username
    ...ProfileCardFragment
  }
`;