Skip to content

Commit

Permalink
Properly kill dev api-server (#11691)
Browse files Browse the repository at this point in the history
Changes are:
- await the process where we kill the server process when restarting. So
that we don't race the new instance of httpServer with existing ones
(while being killed).
- try to gracefully kill the api server, if it's not killed within 2sec,
SIGTERM it.
- unrelated: moved a comment to the right place. I accidentally moved it
wrong place while refactoring in the last PR.
  • Loading branch information
callingmedic911 authored Oct 14, 2024
1 parent 3e556a1 commit b8cd6aa
Show file tree
Hide file tree
Showing 3 changed files with 35 additions and 9 deletions.
3 changes: 3 additions & 0 deletions .changesets/11691.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
- Await and properly kill dev api-server (#11691) by @callingmedic911

Sometime the api-server doesn't get killed in time before the new instance is started. This change makes sure that we wait for the process. If it's not killed within 2 seconds with SIGTERM, we send a SIGKILL to it.
10 changes: 5 additions & 5 deletions packages/api-server/src/buildManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,11 +5,6 @@ export type BuildAndRestartOptions = {
clean?: boolean
}

// We want to delay execution when multiple files are modified on the filesystem,
// this usually happens when running RedwoodJS generator commands.
// Local writes are very fast, but writes in e2e environments are not,
// so allow the default to be adjusted with an env-var.
//
class BuildManager {
private shouldRebuild: boolean
private shouldClean: boolean
Expand All @@ -34,6 +29,11 @@ class BuildManager {
this.shouldClean = false
}
},
// We want to delay execution when multiple files are modified on the filesystem,
// this usually happens when running RedwoodJS generator commands.
// Local writes are very fast, but writes in e2e environments are not,
// so allow the default to be adjusted with an env-var.
//
process.env.RWJS_DELAY_RESTART
? parseInt(process.env.RWJS_DELAY_RESTART, 10)
: 500,
Expand Down
31 changes: 27 additions & 4 deletions packages/api-server/src/serverManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { fork } from 'child_process'
import fs from 'fs'
import path from 'path'

import chalk from 'chalk'
import yargs from 'yargs'
import { hideBin } from 'yargs/helpers'

Expand Down Expand Up @@ -82,13 +83,35 @@ export class ServerManager {
}

async restartApiServer() {
this.killApiServer()
await this.killApiServer()
await this.startApiServer()
}

killApiServer() {
this.httpServerProcess?.emit('exit')
this.httpServerProcess?.kill()
async killApiServer() {
if (!this.httpServerProcess) {
return
}

// Try to gracefully close the server
// If it doesn't close within 2 seconds, forcefully close it
await Promise.race([
new Promise<void>((resolve) => {
console.log(chalk.yellow('Shutting down API server.'))
this.httpServerProcess!.on('exit', () => resolve())
this.httpServerProcess!.kill()
}),
new Promise<void>((resolve) =>
setTimeout(() => {
console.log(
chalk.yellow(
'API server did not exit within 2 seconds, forcefully closing it.',
),
)
this.httpServerProcess!.kill('SIGKILL')
resolve()
}, 2000),
),
])
}
}

Expand Down

0 comments on commit b8cd6aa

Please sign in to comment.