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.
- Related to https://github.com/lerna/lerna/issues/2284
About this issue
- Original URL
- State: closed
- Created 4 years ago
- Reactions: 15
- Comments: 23 (7 by maintainers)
Commits related to this issue
- apollo-server: close connections on ApolloServer.stop() Previously, `ApolloServer.stop()` functioned like `net.Server.close()` in that it did not close idle connections or close active connections af... — committed to apollographql/apollo-server by glasser 3 years ago
- apollo-server: close connections on ApolloServer.stop() Previously, `ApolloServer.stop()` functioned like `net.Server.close()` in that it did not close idle connections or close active connections af... — committed to apollographql/apollo-server by glasser 3 years ago
- apollo-server: close connections on ApolloServer.stop() (#4908) Previously, `ApolloServer.stop()` functioned like `net.Server.close()` in that it did not close idle connections or close active conne... — committed to apollographql/apollo-server by glasser 3 years ago
- chore(deps): bump apollo-datasource from 0.7.2 to 0.7.3 (#160) Bumps apollo-datasource from 0.7.2 to 0.7.3. Changelog Sourced from apollo-datasource's changelog. CHANGELOG The version headers in th... — committed to ProjectXero/dbds by kodiakhq[bot] 3 years ago
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 wayapollo-server-express
works is that it installs middleware on your separately-created Expressapp
and you tell express to listen withapp.listen
. The connection between theApolloServer
object and the Expressapp
is pretty minimal: AS just adds some middleware (request handlers). It’s still your job to stop the HTTP server started byapp.listen
.I think this is kind of confusing! One problem is that ApolloServer doesn’t have a
start()
method that corresponds to thestop()
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:I think adding a
start()
method (which does not start the HTTP server!) will make it more clear that thestop()
method does not stop the HTTP server.(Right now, the equivalent of
start()
gets called automatically from theApolloServer
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
.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.