The server crashes if a promise with deferred handling rejects while another promise is trying to resolve about apollo-server HOT 8 CLOSED

Just FYI, your package-lock.json seems to have been created with some internal corporate npm mirror so that the URLs it tries to install are all from a private mirror. I'll delete the package-lock.json file when trying to reproduce but that might mean I get a different version of packages!

from apollo-server.

I think this is basically working as intended, and it's not actually related to Apollo Server or GraphQL at all, just how Promises/async-await work in Node.JS.

When you write this code:

const myResolverC = async () => {
  const x = thisPromiseWillReject();
  await thisPromiseWillResolveAfterALongTime();
  await x;
}

there is no error handling connected to the x promise until the await x happens. As far as the JS runtime is concerned, nothing in the world is paying attention to the rejection of this Promise (yet!), and so any rejection is considered unhandled. Since Node v15 this will get translated (if you don't set an unhandledRejection handler) into an uncaught exception, which by default crashes your program.

Functions that call multiple Promise-returning functions should use a utility like Promise.all, Promise.allSettled, Promise.race, or Promise.any, all of which ensure that all of the Promises you pass to it get an appropriate error handler attached to it (or use .catch yourself making sure to .catch every Promise before waiting on any of them).

from apollo-server.

To demonstrate that this is unrelated to AS or graphql, replace your server.mjs with:

const REJECTION_TIMER_MILLIS = 1000;
const RESOLUTION_TIMER_MILLIS = 3000;

const thisPromiseWillReject = () => {
  return new Promise((_, reject) => setTimeout(() => {
    reject(new Error('I am rejecting.'));
  }, REJECTION_TIMER_MILLIS));
}

const thisPromiseWillResolveAfterALongTime = () => {
  return new Promise((resolve) => setTimeout(resolve, RESOLUTION_TIMER_MILLIS));
}

const myResolverC = async () => {
  const x = thisPromiseWillReject();
  await thisPromiseWillResolveAfterALongTime();
  await x;
}


try {
  await myResolverC();
} catch (e) {
  console.log(`Error caught from await: ${e}`)
}

Running this shows:

$ node server.mjs 
file:///Users/glasser/Projects/Apollo/mock-apollo-server-for-issue/server.mjs:6
    reject(new Error('I am rejecting.'));
           ^

Error: I am rejecting.
    at Timeout._onTimeout (file:///Users/glasser/Projects/Apollo/mock-apollo-server-for-issue/server.mjs:6:12)
    at listOnTimeout (node:internal/timers:573:17)
    at process.processTimers (node:internal/timers:514:7)

Node.js v21.6.2

Note that you do not get the "Error caught from await" message here; this is the same issue, demonstrated without any use of @apollo/server or graphql.

from apollo-server.

Just FYI, your package-lock.json seems to have been created with some internal corporate npm mirror so that the URLs it tries to install are all from a private mirror. I'll delete the package-lock.json file when trying to reproduce but that might mean I get a different version of packages!

thanks for the heads up, I replaced the repro with a public one

from apollo-server.

@glasser I totally agree that uncaught exceptions should not exist in the app in the first place, but my concern comes from apollo-server being able to handle some uncaught exceptions, but crashing on others.

As you noted, the example I provided has an uncaught exception:

const myResolverC = async () => {
  const x = thisPromiseWillReject();
  await thisPromiseWillResolveAfterALongTime();
  await x;
}

because myResolverC is not handled until await happens. This turns into an uncaught exception as you noted, which crashes the app.

But this resolver is handled totally fine by apollo server:

const myFailingResolver = async () => {
  throw new Error('yikes');
}

Even though it's the same end result (uncaught exception). Maybe I'm missing something? It would be ideal if all unhandled exceptions were treated the same way by apollo-server.

from apollo-server.

My point is that Apollo Server has nothing to do with this Promise. The very thing that (in a sense) makes Apollo Server/graphql-js aware that this Promise is connected to the result of executing your resolver in any way is the fact that it gets a .catch or await applied to it while the function is running. As far as Apollo Server is concerned, this Promise might have been constructed in some completely unrelated piece of code, say some background setInterval or anything else. Whenever you create a Promise in JS, it's your job to tell Node that you're paying attention to rejections on that Promise before you yield to the event loop (by awaiting, say), not afterwards. This is what functions like Promise.all are for. Apollo Server doesn't somehow hijack the creation of Promises within resolvers to do something extra fancy to them to handle their rejections — you're expected to set your Promises up properly like in any other async code.

As far as Node knows, your function may have just been:

const myResolverC = async () => {
  const x = thisPromiseWillReject();
  await thisPromiseWillResolveAfterALongTime();
}

because it never actually gets to the point of running your await x before x rejects. And since there's no await here, it's not correct for myResolverC's returned Promise to reject just because x rejects — you haven't told the JS runtime to do that!

from apollo-server.

I see what the difference is now -- throw Error is picked up by the async resolver function as a promise rejection, but a promise with no await is just dangling in a sense so the async resolver function would never see it as you noted, and just resolve to undefined.

Thanks for explaining that to me, I will relay it.

from apollo-server.