Skip to content

Commit e77c0bc

Browse files
authored
s3: fix SIGINT handling (#1653)
* don't short-circuit uploader destruction * allow time for uploaders to shutdown before propagating signals Closes #1417
1 parent 18b39e3 commit e77c0bc

File tree

2 files changed

+24
-4
lines changed

2 files changed

+24
-4
lines changed

lib/external/s3.js

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -28,15 +28,20 @@ const init = (config) => {
2828
const inflight = new Set();
2929
function destroy() {
3030
destroyed = true;
31-
return new Promise((resolve, reject) => {
31+
return new Promise(resolve => {
3232
let remaining = 0;
3333
for (const req of inflight) {
3434
++remaining; // eslint-disable-line no-plusplus
3535
req.once('close', () => { // eslint-disable-line no-loop-func
3636
if (!--remaining) resolve(); // eslint-disable-line no-plusplus
3737
});
38-
req.once('error', reject);
39-
req.destroy(new Error('Aborted by request'));
38+
39+
const destroyError = new Error('Aborted by request');
40+
req.once('error', err => {
41+
// eslint-disable-next-line no-console
42+
if (err !== destroyError) console.error('Ignoring unexpected error:', err);
43+
});
44+
req.destroy(destroyError);
4045
}
4146
});
4247
}

lib/task/s3.js

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -65,19 +65,34 @@ const uploadPending = withContainer(({ s3, Blobs }) => async (limit) => {
6565

6666
const signals = ['SIGINT', 'SIGTERM'];
6767

68+
let uploader;
69+
6870
const shutdownListener = async signal => {
6971
try {
7072
await s3.destroy();
7173
} catch (err) {
7274
console.log('s3 threw error while shutting down; it will be ignored:', err);
7375
}
76+
77+
if (uploader) {
78+
// Wait a reasonable amount of time for the uploader to complete
79+
try {
80+
await Promise.race([
81+
uploader,
82+
new Promise(resolve => { setTimeout(resolve, 10_000); }),
83+
]);
84+
} catch (err) {
85+
console.error('Unexpected error from s3 uploader:', err);
86+
}
87+
}
88+
7489
process.kill(process.pid, signal);
7590
};
7691
signals.forEach(s => process.once(s, shutdownListener));
7792

7893
try {
7994
console.log(`Uploading ${count ?? 'all'} blobs...`);
80-
await Blobs.s3UploadPending(limit);
95+
await (uploader = Blobs.s3UploadPending(limit));
8196
console.log(`[${new Date().toISOString()}]`, 'Upload completed.');
8297
} finally {
8398
signals.forEach(s => process.removeListener(s, shutdownListener));

0 commit comments

Comments
 (0)