Skip to content

Commit

Permalink
fix(StdioClient): Restart after premature exist of child server process
Browse files Browse the repository at this point in the history
Closes #42.
  • Loading branch information
nokome committed Dec 13, 2019
1 parent f40b8f2 commit eddeb0e
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 20 deletions.
15 changes: 10 additions & 5 deletions src/stdio/StdioClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -55,15 +55,17 @@ export class StdioClient extends StreamClient {
)
// This event can happen at start up before `super()` is called
// and `this` is defined. There is not a better way to test for
// success of startup.
if (this !== undefined) this.child = undefined
// success of startup other than `this !== undefined`
if (this !== undefined) {
this.stop().catch(error => log.error(error))
}
})

child.on('exit', (code: number | null, signal: string | null) => {
log.error(
`Server exited prematurely with exit code ${code} and signal ${signal}`
)
this.child = undefined
this.stop().catch(error => log.error(error))
})

const { stdin, stdout, stderr } = child
Expand All @@ -79,17 +81,20 @@ export class StdioClient extends StreamClient {
}

/**
* @override Overrider of {@link Executor.stop} to
* @override Override of {@link Executor.stop} to
* stop the child server process.
*/
public stop(): Promise<void> {
log.debug(`Stopping StdioServer`)

if (this.child !== undefined) {
// Avoid unnecessary log errors by removing listener
this.child.removeAllListeners('exit')
this.child.kill()
this.child = undefined
}
return Promise.resolve()

return super.stop()
}

/**
Expand Down
31 changes: 17 additions & 14 deletions src/stdio/StdioClientServer.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,28 +33,35 @@ describe('StdioClient and StdioServer', () => {

await testClient(client)

// Do not await the next two calls to `decode` - they do not
// resolve due to the bad message

// Test the server crashing
const messages = nextClientMessages()
client.decode('send bad message').catch(error => {
client.decode('crash now!').catch(error => {
throw error
})
expect((await messages)[0]).toMatch(/^Error parsing message as JSON: ah hah/)
expect((await messages)[0]).toMatch(
/^Server exited prematurely with exit code 1 and signal null/
)

// The server should get restarted JIT on next
// method request
expect(await client.decode('1', 'json')).toEqual(1)

await client.stop()
})

test('crash', async () => {
const client = new StdioClient(testServer())

await client.start()

// Test the server sending a bad message
const messages = nextClientMessages()
client.decode('crash now!').catch(error => {
// Do not await`decode` - it does not
// resolve due to the bad message
client.decode('send bad message').catch(error => {
throw error
})
expect((await messages)[0]).toMatch(
/^Server exited prematurely with exit code 1 and signal null/
/^Error parsing message as JSON: ah hah/
)

await client.stop()
Expand All @@ -71,9 +78,7 @@ describe('StdioClient and StdioServer', () => {
await client.start()

const messages = await nextMessages
expect(messages[0]).toMatch(
/^Starting StdioServer/
)
expect(messages[0]).toMatch(/^Starting StdioServer/)
expect(messages[1]).toMatch(
/^Server exited prematurely with exit code 1 and signal null/
)
Expand All @@ -88,9 +93,7 @@ describe('StdioClient and StdioServer', () => {
await client.start()

const messages = await nextMessages
expect(messages[0]).toMatch(
/^Starting StdioServer/
)
expect(messages[0]).toMatch(/^Starting StdioServer/)
expect(messages[1]).toMatch(
/^Server exited prematurely with exit code 0 and signal null/
)
Expand Down
14 changes: 13 additions & 1 deletion src/stream/StreamClient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ export class StreamClient extends Client {
}

/**
* Create an instance of `StreamClient`.
* Start the client.
*
* @param {Writable} outgoing Outgoing stream to send JSON-RPC requests on.
* @param {Readable} incoming Incoming stream to receive JSON-RPC responses on.
Expand Down Expand Up @@ -53,6 +53,18 @@ export class StreamClient extends Client {
return Promise.resolve()
}

/**
* Stop the client.
*
* Sets `this.encoder` to `undefined` so that it gets
* reconstructed with a new stream if `start()` is called later.
*/
public stop(): Promise<void> {
this.encoder = undefined

return Promise.resolve()
}

/**
* @implements Implements {@link Client.send} to start the
* client if necessary and write the request to the encoder.
Expand Down

0 comments on commit eddeb0e

Please sign in to comment.