Fix Docker tunnel pull race condition

Previously we assumed that when pull() resolved it was complete. This is
false. In reality we have to wait for the resulting stream to explicitly
finish, so we had a subtle race condition here where the tunnel would
not be available if it was required immediately after startup (e.g. in
the tests) on the first run of the app, on a weak connection or whilst
Docker hub was running a little slow.

Author: pimterry

src/interceptors/docker/docker-tunnel-proxy.ts

@@ -7,13 +7,18 @@ import { getDockerHostAddress, isImageAvailable } from './docker-commands';
 import { isDockerAvailable } from './docker-interception-services';
 import { delay } from '../../util/promise';
 import { reportError } from '../../error-tracking';
+import { waitForDockerStream } from './docker-utils';
 
 const DOCKER_TUNNEL_IMAGE = "httptoolkit/docker-socks-tunnel:v1.1.0";
 const DOCKER_TUNNEL_LABEL = "tech.httptoolkit.docker.tunnel";
 
 const getDockerTunnelContainerName = (proxyPort: number) =>
     `httptoolkit-docker-tunnel-${proxyPort}`;
 
+const pullTunnelImage = (docker: Docker) =>
+    docker.pull(DOCKER_TUNNEL_IMAGE)
+    .then(stream => waitForDockerStream(docker, stream));
+
 // Parallel mutation of a single Docker container's state is asking for trouble, so we use
 // a simple lock over all operations (across all proxes, not per-proxy, just for simplicity/safety).
 const containerMutex = new Mutex();
@@ -25,7 +30,7 @@ export async function prepareDockerTunnel() {
     await containerMutex.runExclusive(async () => {
         const docker = new Docker();
         if (await isImageAvailable(docker, DOCKER_TUNNEL_IMAGE)) return;
-        else await docker.pull(DOCKER_TUNNEL_IMAGE).catch(console.warn);
+        else await pullTunnelImage(docker).catch(console.warn);
     });
 }
 
@@ -45,7 +50,7 @@ export function ensureDockerTunnelRunning(proxyPort: number) {
 
         // Make sure we have the image available (should've been pre-pulled, but just in case)
         if (!await docker.getImage(DOCKER_TUNNEL_IMAGE).inspect().catch(() => false)) {
-            await docker.pull(DOCKER_TUNNEL_IMAGE);
+            await pullTunnelImage(docker);
         }
 
         // Ensure we have a ready-to-use container here:
@@ -195,7 +200,7 @@ export async function getDockerTunnelPort(proxyPort: number): Promise<number> {
     return portCache[proxyPort]!;
 }
 
-export async function refreshDockerTunnelPortCache(proxyPort: number, { force } = { force: false }): Promise<number> {
+async function refreshDockerTunnelPortCache(proxyPort: number, { force } = { force: false }): Promise<number> {
     console.log("Querying Docker tunnel port...");
     try {
         if (!force && _.isObject(portCache[proxyPort])) {

src/interceptors/docker/docker-utils.ts

@@ -0,0 +1,18 @@
+import * as Docker from 'dockerode';
+
+export const waitForDockerStream = (
+    docker: Docker,
+    stream: NodeJS.ReadableStream
+) => new Promise<void>((resolve, reject) => {
+    docker.modem.followProgress(stream, (
+        err: Error | null,
+        stream: Array<{ error?: string }>
+    ) => {
+        if (err) reject(err);
+
+        const firstError = stream.find((msg) => !!msg.error);
+        if (firstError) reject(new Error(firstError.error));
+
+        resolve();
+    });
+});

test/interceptors/docker-attachment.spec.ts

@@ -7,8 +7,9 @@ import { expect } from 'chai';
 import * as Docker from 'dockerode';
 import fetch from 'node-fetch';
 
-import { setupInterceptor, itIsAvailable } from './interceptor-test-utils';
 import { delay } from '../../src/util/promise';
+import { setupInterceptor, itIsAvailable } from './interceptor-test-utils';
+import { waitForDockerStream } from '../../src/interceptors/docker/docker-utils';
 
 const docker = new Docker();
 const DOCKER_FIXTURES = path.join(__dirname, '..', 'fixtures', 'docker');
@@ -39,17 +40,7 @@ async function buildAndRun(dockerFolder: string, options: {
         if (data.stream) console.log(data.stream.replace(/\n$/, ''));
     });
 
-    // Wait for the build to complete without errors:
-    await new Promise<void>((resolve, reject) => {
-        docker.modem.followProgress(buildStream, (err: Error | null, stream: Array<{ error?: string }>) => {
-            if (err) reject(err);
-
-            const firstError = stream.find((msg) => !!msg.error);
-            if (firstError) reject(new Error(firstError.error));
-
-            resolve();
-        });
-    });
+    await waitForDockerStream(docker, buildStream);
     console.log(`${dockerFolder} image built`);
 
     // Run the container, using its default entrypoint