getDataFromTree Promise is always resolved

[kelly-tock]

Intended outcome:

I would like to be able to somehow have the Promise returned from getDataFromTree reject if a query fails. This will allow in SSR to send the error to the error handling middleware.

Actual outcome:

Even though the query is failing with a 403 error in my case, getDataFromTree is still resolving with the rendered markup. Here is an example of the results i'm seeing in the console for the error:

Error [ServerError]: Response not successful: Received status code 500
    at Object.throwServerError (/app/node_modules/@apollo/client/link/utils/throwServerError.js:2:17)
    at /app/node_modules/@apollo/client/link/http/parseAndCheckHttpResponse.js:21:13
    at processTicksAndRejections (internal/process/task_queues.js:93:5) {
  response: Response {
    size: 0,
    timeout: 0,
    [Symbol(Body internals)]: { body: [PassThrough], disturbed: true, error: null },
    [Symbol(Response internals)]: {
      url: 'https://staging.exploretock.com/api/graphql/Business',
      status: 500,
      statusText: 'Internal Server Error',
      headers: [Headers],
      counter: 0
    }
  },
  statusCode: 500,
  result: {
    error: {
      errorCode: 5000,
      httpStatusCode: 403,
      classification: 'PERMANENT',
      message: 'Forbidden'
    }
  }
}

Here is my paired down ssr code:

export const serverRender = async ({
  req,
  res,
  next,
  styleSheetPath,
  environment,
}: ServerRenderOptions) => {
  initializeLogger(environment);

  const { store } = res.locals;

  const clientHeaders = getClientHeaders(req);

  const apolloClient = initializeApolloClient(store, clientHeaders);

  try {
    const getApp = ({ disableGeneration }: { disableGeneration: boolean }) => {
      const App = (
          <ApolloProvider client={apolloClient}>
            <StaticRouter location={req.url} context={routerContext}>
              <HelmetProvider context={helmetContext}>
                <Application disableGeneration={disableGeneration} />
              </HelmetProvider>
            </StaticRouter>
          </ApolloProvider>
      );
      return App;
    };

    // Here, we render the app once with apollo, and disable stylesheet generation for mui
    // If we didn't do this, the stylesheets would mismatch always from server to client.
      await getDataFromTree(getApp({ disableGeneration: true }));

    const initialApolloState = apolloClient.extract();

} catch (err) {
    // pass to error handler
    next(err);
  }

How to reproduce the issue:

Versions

System:
    OS: macOS 11.5.2
  Binaries:
    Node: 14.16.0 - ~/.nvm/versions/node/v14.16.0/bin/node
    Yarn: 1.22.5 - ~/.yarn/bin/yarn
    npm: 6.14.11 - ~/.nvm/versions/node/v14.16.0/bin/npm
  Browsers:
    Chrome: 93.0.4577.63
    Edge: 93.0.961.44
    Firefox: 88.0
    Safari: 14.1.2
  npmPackages:
    @apollo/client: 3.3.15 => 3.3.15 

[wintercounter]

I don't think error handling is the responsibility of getDataFromTree. That would make your app act differently server-side than on client-side. I believe you should solve this by using a ErrorBoundary, 2 separate ones for client/server rendering, so you can control the outcome on both sides.

[miqdadfwz]

I was wondering which commits causing this new behavior? To be clear I'm not saying this behavior is bad, and I 100% agree with the new design, but my repository is really depends on catch block whenever the getDataFromTree fails and refactor it is really time consuming and not worth of our effort. I have been looking to the commits to get better understanding of the context. It would be better if there is a reliable workaround to get the old behavior back in Apollo v3.

[richardscarrott]

@miqdadfwz this has come as a surprise to us as we too were relying on it rejecting as per AC2.

Did you figure out where the best place to log API errors during SSR is?