apollo-server: In the batteries-included `apollo-server`, ApolloServer.stop() can hang indefinitely waiting for connections to close (just like net.Server.close)

Description

I’ve been trying to reproduce an issue where the server process appears to be orphaned and continues running despite being killed.

I’m using tsc-watch to watch for code changes and when a file change is detected it sends SIGTERM to the process and then recreates it by invoking node ./build again.

However in this scenario some simple code in my application is unexpectedly causing the process to be orphaned and never exit as multiple processes are created all vying for the same port.

Version Info

apollo-server: 2.13.0

Expected Behavior

The server stops when await server.stop() is called

Actual Behavior

The server does not stop and the call to server never finishes, the process never exits.

Reproduction

https://github.com/justinmchase/apollo-lerna-repro

Steps

npm i
npm start
# open http://localhost:3000 in browser

This will cause a simple react page to continually call the api. If you open this file: https://github.com/justinmchase/apollo-lerna-repro/blob/master/packages/api/src/index.ts

And simply save it you will see the server restart seamlessly.

Now go and uncomment this line: https://github.com/justinmchase/apollo-lerna-repro/blob/master/packages/api/src/index.ts#L61

And observe that the call to await server.stop() does not stop the server and never returns and the process is never exited.

About this issue

  • Original URL
  • State: closed
  • Created 4 years ago
  • Reactions: 15
  • Comments: 23 (7 by maintainers)

Commits related to this issue

Most upvoted comments

After testing this extensively I think that all the complexity of this sample can be ignored and simply starting the server and calling await server.stop() is the only problem. The server does not alway stop and the time it can take to stop is highly variable and it can actually continue to accept and process new incoming connections while hung in the stop method.

If I simply close the process then the server does stop (of course) but it will abruptly end all ongoing requests mid stream.

What I expect to happen is for the server to immediately stop accepting incoming connections and once all ongoing connections are completed then the call to stop resolves. If there are no currently processing requests then it would end immediately.

Check out #4908.

@bigman73 ApolloServer.stop() only stops the lifecycle of code that’s specifically to the apollo/graphql server itself: eg, any plugins, signal handlers, etc. But the way apollo-server-express works is that it installs middleware on your separately-created Express app and you tell express to listen with app.listen. The connection between the ApolloServer object and the Express app is pretty minimal: AS just adds some middleware (request handlers). It’s still your job to stop the HTTP server started by app.listen.

I think this is kind of confusing! One problem is that ApolloServer doesn’t have a start() method that corresponds to the stop() method. This leads to other issues too; for example, some errors that can occur asynchronously during startup (eg, errors from plugins or from loading a federated schema) aren’t easily handled by your program. I am planning to very soon introduce a (optional for now) start API, so basic Express usage would look like:

  const apolloServer = new ApolloServer({
    typeDefs,
    resolvers
  });
  const app = express();
  apolloServer.applyMiddleware({ app });
  await apolloServer.start();
  const httpServer = app.listen({ port: 4000 })

  // later
  await new Promise(resolve => httpServer.close(resolve));  // ... though look a few comments down for some caveats!
  await apolloServer.stop();

I think adding a start() method (which does not start the HTTP server!) will make it more clear that the stop() method does not stop the HTTP server.

(Right now, the equivalent of start() gets called automatically from the ApolloServer constructor and there’s no easy way to await its successful result and handle any errors.)

Still running into this issue, noticed it when using ts-node-dev.

[watch:server ] [DEBUG] 16:34:52 Removing all watchers from files
[watch:server ] [DEBUG] 16:34:52 Child is still running, restart upon exit
[watch:server ] [DEBUG] 16:34:52 Disconnecting from child
[watch:server ] [DEBUG] 16:34:52 Sending SIGTERM kill to child pid 31733
[watch:server ] [DEBUG] 16:34:59 Child exited with code 0
[watch:server ] [DEBUG] 16:34:59 Starting child process -r /tmp/ts-node-dev-hook-7320061111056044.js /home/chance/test/api/node_modules/ts-node-dev/lib/wrap.js src/index.ts
[watch:server ] [DEBUG] 16:34:59 /home/chance/test/api/src/index.ts added to watcher
[watch:server ] [DEBUG] 16:34:59 /home/chance/test/api/src/schema.ts added to watcher
[watch:server ] 🚀  Server ready at http://localhost:4000/

It sometimes immediately restarts, and sometimes there is (like above) a random delay between sending SIGTERM and it actually restarting.

Correction, I’m seeing it take 30s-2m to complete the call to stop. I wasn’t patient enough but it does seem to eventually stop.

That is still surprisingly long and I feel like there is something wrong but it does eventually stop. I don’t mind it actually waiting for current connections to complete but it does appear to keep accepting new connections which seems bad.

EDIT: Correction to the correction, sometimes it stops sometimes even after 5m its still running.