Skip to content

Commit e7e16b6

Browse files
pimterrypimterry
committed
Improve shutdown process, using the server's new shutdown API
This allows us to do clean shutdowns on Windows, where signals otherwise don't exist. That's useful for UX generally to close up intercepted apps etc, but is especially important to clean up Docker containers etc automatically.
1 parent 1c65a8a commit e7e16b6

File tree

3 files changed

+166
-59
lines changed

3 files changed

+166
-59
lines changed

‎src/errors.ts‎

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,36 @@
1+
const DEV_MODE = process.env.HTK_DEV === 'true';
2+
3+
import * as Sentry from '@sentry/electron';
4+
import { RewriteFrames } from '@sentry/integrations';
5+
6+
if (!DEV_MODE) {
7+
Sentry.init({
8+
dsn: 'https://[email protected]/1367048',
9+
integrations: [
10+
new RewriteFrames({
11+
// Make all paths relative to this root, because otherwise it can make
12+
// errors unnecessarily distinct, especially on Windows.
13+
root: process.platform === 'win32'
14+
// Root must always be POSIX format, so we transform it on Windows:
15+
? __dirname
16+
.replace(/^[A-Z]:/, '') // remove Windows-style prefix
17+
.replace(/\\/g, '/') // replace all `\\` instances with `/`
18+
: __dirname
19+
})
20+
]
21+
});
22+
}
23+
24+
export function reportError(error: Error | string) {
25+
console.log(error);
26+
27+
if (typeof error === 'string') {
28+
Sentry.captureMessage(error);
29+
} else {
30+
Sentry.captureException(error);
31+
}
32+
}
33+
34+
export function addBreadcrumb(breadcrumb: Sentry.Breadcrumb) {
35+
Sentry.addBreadcrumb(breadcrumb);
36+
}

‎src/index.ts‎

Lines changed: 19 additions & 59 deletions
Original file line numberDiff line numberDiff line change
@@ -1,37 +1,9 @@
11
const DEV_MODE = process.env.HTK_DEV === 'true';
22

3-
import * as Sentry from '@sentry/electron';
4-
import { RewriteFrames } from '@sentry/integrations';
5-
6-
if (!DEV_MODE) {
7-
Sentry.init({
8-
dsn: 'https://[email protected]/1367048',
9-
integrations: [
10-
new RewriteFrames({
11-
// Make all paths relative to this root, because otherwise it can make
12-
// errors unnecessarily distinct, especially on Windows.
13-
root: process.platform === 'win32'
14-
// Root must always be POSIX format, so we transform it on Windows:
15-
? __dirname
16-
.replace(/^[A-Z]:/, '') // remove Windows-style prefix
17-
.replace(/\\/g, '/') // replace all `\\` instances with `/`
18-
: __dirname
19-
})
20-
]
21-
});
22-
}
23-
24-
function reportError(error: Error | string) {
25-
console.log(error);
3+
// Set up error handling before everything else:
4+
import { reportError, addBreadcrumb } from './errors';
265

27-
if (typeof error === 'string') {
28-
Sentry.captureMessage(error);
29-
} else {
30-
Sentry.captureException(error);
31-
}
32-
}
33-
34-
import { spawn, exec, ChildProcess } from 'child_process';
6+
import { spawn, ChildProcess } from 'child_process';
357
import * as os from 'os';
368
import { promises as fs } from 'fs'
379
import * as net from 'net';
@@ -44,6 +16,7 @@ import * as uuid from 'uuid/v4';
4416
import * as yargs from 'yargs';
4517
import * as semver from 'semver';
4618
import * as rimraf from 'rimraf';
19+
const rmRF = promisify(rimraf);
4720
import * as windowStateKeeper from 'electron-window-state';
4821
import { getSystemProxy } from 'os-proxy-config';
4922

@@ -55,8 +28,7 @@ registerContextMenu({
5528
import { reportStartupEvents } from './report-install-event';
5629
import { getMenu, shouldAutoHideMenu } from './menu';
5730
import { getDeferred, delay } from './util';
58-
59-
const rmRF = promisify(rimraf);
31+
import { stopServer } from './stop-server';
6032

6133
const packageJson = require('../package.json');
6234

@@ -166,34 +138,21 @@ if (!amMainInstance) {
166138
}
167139

168140
let serverKilled = false;
169-
app.on('will-quit', (event) => {
141+
app.on('will-quit', async (event) => {
170142
if (server && !serverKilled) {
143+
// Don't shutdown until we've tried to kill the server
144+
event.preventDefault();
145+
171146
serverKilled = true;
147+
172148
try {
173-
if (isWindows) {
174-
// Don't shutdown until we've tried to kill the server
175-
event.preventDefault();
176-
177-
// Forcefully kill the pid (the cmd) and child processes
178-
exec(`taskkill /pid ${server.pid} /T /F`, (error, stdout, stderr) => {
179-
if (error) {
180-
console.log(stdout);
181-
console.log(stderr);
182-
reportError(error);
183-
}
184-
185-
// We've done our best - now shut down for real. Disable errors, otherwise
186-
// we can receive reports for invisible errors during/just after exit.
187-
app.quit();
188-
});
189-
} else {
190-
// Make sure we clean up the whole group (shell + node).
191-
// https://azimi.me/2014/12/31/kill-child_process-node-js.html
192-
process.kill(-server.pid!);
193-
}
149+
await stopServer(server, AUTH_TOKEN);
150+
// We've done our best - now shut down for real.
151+
app.quit();
194152
} catch (error) {
195153
console.log('Failed to kill server', error);
196154
reportError(error);
155+
app.quit();
197156
}
198157
}
199158
});
@@ -398,13 +357,13 @@ if (!amMainInstance) {
398357
stderr.pipe(process.stderr);
399358

400359
server.stdout!.on('data', (data) => {
401-
Sentry.addBreadcrumb({ category: 'server-stdout', message: data.toString('utf8'), level: <any>'info' });
360+
addBreadcrumb({ category: 'server-stdout', message: data.toString('utf8'), level: <any>'info' });
402361
});
403362

404363
let lastError: string | undefined = undefined;
405364
stderr.on('data', (data) => {
406365
const errorOutput = data.toString('utf8');
407-
Sentry.addBreadcrumb({ category: 'server-stderr', message: errorOutput, level: <any>'warning' });
366+
addBreadcrumb({ category: 'server-stderr', message: errorOutput, level: <any>'warning' });
408367

409368
// Remember the last '*Error:' line we saw.
410369
lastError = errorOutput
@@ -434,7 +393,7 @@ if (!amMainInstance) {
434393
error = new Error(`Server shutdown unexpectedly with code ${errorOrCode}`);
435394
}
436395

437-
Sentry.addBreadcrumb({ category: 'server-exit', message: error.message, level: <any>'error', data: { serverRunTime } });
396+
addBreadcrumb({ category: 'server-exit', message: error.message, level: <any>'error', data: { serverRunTime } });
438397
reportError(error);
439398

440399
showErrorAlert(
@@ -480,7 +439,8 @@ if (!amMainInstance) {
480439
});
481440

482441
// Check we're happy using the default proxy settings
483-
getSystemProxy().then((proxyConfig) => {
442+
getSystemProxy()
443+
.then((proxyConfig) => {
484444
let shouldDisableProxy = false;
485445

486446
if (proxyConfig) {

‎src/stop-server.ts‎

Lines changed: 111 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,111 @@
1+
import * as ChildProcess from 'child_process';
2+
import * as http from 'http';
3+
4+
import { reportError } from './errors';
5+
import { delay } from './util';
6+
7+
const isRunning = (pid: number) => {
8+
try {
9+
process.kill(pid, 0);
10+
return true;
11+
} catch (e) {
12+
if (e.code === 'ESRCH') return false;
13+
else throw e;
14+
}
15+
}
16+
17+
export async function stopServer(proc: ChildProcess.ChildProcess, token: string) {
18+
await softShutdown(token)
19+
.catch(reportError); // If that fails, continue shutting down anyway
20+
21+
// In each case, that triggers a clean shutdown. We want to make sure it definitely shuts
22+
// down though, so we poll the process state, and kill it if it's still running in 3 seconds.
23+
24+
const deadline = Date.now() + 3000;
25+
26+
do {
27+
await delay(100);
28+
29+
if (Date.now() >= deadline) {
30+
await hardKill(proc);
31+
break;
32+
}
33+
} while (isRunning(proc.pid!))
34+
}
35+
36+
function softShutdown(token: string) {
37+
// We first try to cleanly shut down the server, so it can clean up after itself.
38+
// On Mac & Linux, we could shut down the server with SIGTERM, with some fiddling to detach it
39+
// so that we kill the full shell script + node tree. On Windows that's not possible though,
40+
// because Windows doesn't support signals at all, and even workarounds to inject SIGINT don't
41+
// seem to work properly from Electron.
42+
43+
// To handle all this, we send a HTTP request to the GraphQL API instead, which triggers the same thing.
44+
return new Promise<void>((resolve, reject) => {
45+
const req = http.request("http://127.0.0.1:45457", {
46+
method: 'POST',
47+
headers: {
48+
'content-type': 'application/json',
49+
'origin': 'https://app.httptoolkit.tech',
50+
'authorization': `Bearer ${token}`
51+
}
52+
});
53+
req.on('error', reject);
54+
55+
req.end(JSON.stringify({
56+
operationName: 'Shutdown',
57+
query: 'mutation Shutdown { shutdown }',
58+
variables: {}
59+
}));
60+
61+
req.on('response', (res) => {
62+
if (res.statusCode !== 200) {
63+
reject(new Error(`Shutdown request received unexpected ${res.statusCode} response`));
64+
return;
65+
}
66+
67+
const responseChunks: Buffer[] = [];
68+
res.on('data', (data) => responseChunks.push(data));
69+
res.on('error', reject);
70+
res.on('end', () => {
71+
const rawResponseBody = Buffer.concat(responseChunks);
72+
try {
73+
const responseBody = JSON.parse(rawResponseBody.toString('utf8'));
74+
const errors = responseBody.errors as Array<{ message: string, path: string[] }> | undefined;
75+
if (errors?.length) {
76+
console.error(errors);
77+
const errorCount = errors.length > 1 ? `s (${errors.length})` : '';
78+
79+
throw new Error(
80+
`Server error${errorCount} during shutdown: ${errors.map(e =>
81+
`${e.message} at ${e.path.join('.')}`
82+
).join(', ')}`
83+
);
84+
}
85+
86+
resolve();
87+
} catch (e) {
88+
reject(e);
89+
}
90+
});
91+
});
92+
});
93+
}
94+
95+
async function hardKill(proc: ChildProcess.ChildProcess) {
96+
if (process.platform !== "win32") {
97+
process.kill(-proc.pid!, 'SIGKILL');
98+
} else {
99+
return new Promise<void>((resolve, reject) => {
100+
ChildProcess.exec(`taskkill /pid ${proc.pid} /T /F`, (error, stdout, stderr) => {
101+
if (error) {
102+
console.log(stdout);
103+
console.log(stderr);
104+
reject(error);
105+
} else {
106+
resolve();
107+
}
108+
});
109+
});
110+
}
111+
}

0 commit comments

Comments
 (0)