Skip to content

Commit

Permalink
Fix disposing the docker daemon request on non-network errors
Browse files Browse the repository at this point in the history
Change-type: patch
  • Loading branch information
thgreasi committed Nov 9, 2023
1 parent db962ac commit aa506a6
Show file tree
Hide file tree
Showing 2 changed files with 24 additions and 7 deletions.
3 changes: 0 additions & 3 deletions .mocharc.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,4 @@
module.exports = {
timeout: 25000,
spec: 'test/**/*.spec.ts',
// TODO: This shouldn't be necessary, but the tests on node20 hang, while they pass on node 16 & 18.
// `leaked-handles` showed an unclosed docker deamon connection.
exit: true,
};
28 changes: 24 additions & 4 deletions lib/build/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ import * as fs from 'mz/fs';
import * as path from 'path';
import { Duplex } from 'stream';
import * as tar from 'tar-stream';
import type * as http from 'http';

// Import hook definitions
import * as Plugin from './plugin';
Expand Down Expand Up @@ -83,6 +84,7 @@ export default class Builder {
const failBuild = _.once((err: Error) => {
streamError = err;
dup.destroy(err);
cleanupDaemonStream?.();
return this.callHook(
hooks,
'buildFailure',
Expand All @@ -96,6 +98,8 @@ export default class Builder {
inputStream.on('error', failBuild);
dup.on('error', failBuild);

let cleanupDaemonStream: (() => void) | undefined;

const buildPromise = (async () => {
const daemonStream = await this.docker.buildImage(inputStream, buildOpts);

Expand All @@ -104,7 +108,24 @@ export default class Builder {
daemonStream,
layers,
fromTags,
reject,
(err) => {
cleanupDaemonStream = () => {
if ('req' in daemonStream!
&& daemonStream.req != null
&& typeof daemonStream.req === 'object'
&& 'destroy' in daemonStream.req
&& typeof daemonStream.req.destroy === 'function'
) {
const req = (daemonStream.req as http.ClientRequest);
if (!req.destroyed ) {
req.destroy();
}
}
daemonStream.unpipe();
cleanupDaemonStream = undefined;
};
reject(err)
},
);
outputStream.on('error', (error: Error) => {
daemonStream.unpipe();
Expand Down Expand Up @@ -174,7 +195,7 @@ export default class Builder {
return await Promise.all([relPath, fs.stat(file), fs.readFile(file)]);
}),
);
await fileInfos.map(async (fileInfo: [string, fs.Stats, Buffer]) => {
await Promise.all(fileInfos.map(async (fileInfo: [string, fs.Stats, Buffer]) => {
await new Promise<void>((resolve, reject) =>
pack.entry(
{ name: fileInfo[0], size: fileInfo[1].size },
Expand All @@ -188,7 +209,7 @@ export default class Builder {
},
),
);
});
}));
// Tell the tar stream we're done
pack.finalize();
// Create a build stream to send the data to
Expand Down Expand Up @@ -275,7 +296,6 @@ function getDockerDaemonBuildOutputParserStream(
this.emit('data', data.stream);
}
} catch (error) {
daemonStream.unpipe();
onError(error);
}
}),
Expand Down

0 comments on commit aa506a6

Please sign in to comment.