fix: reconnect to /events if disconnected (podman-desktop#4809)

* fix: reconnect to /events if disconnected fixes podman-desktop#4712 Signed-off-by: Florent Benoit <[email protected]>
feloy · Nov 17, 2023 · 83630c2 · 83630c2
1 parent 1d4e9bd
commit 83630c2
Show file tree

Hide file tree

Showing 2 changed files with 132 additions and 2 deletions.
diff --git a/packages/main/src/plugin/container-registry.spec.ts b/packages/main/src/plugin/container-registry.spec.ts
@@ -144,6 +144,10 @@ class TestContainerProviderRegistry extends ContainerProviderRegistry {
   setStreamsPerContainerId(id: string, data: NodeJS.ReadWriteStream) {
     this.streamsPerContainerId.set(id, data);
   }
+
+  setRetryDelayEvents(delay: number): void {
+    super.retryDelayEvents = delay;
+  }
 }
 
 let containerRegistry: TestContainerProviderRegistry;
@@ -2210,3 +2214,87 @@ test('createNetwork', async () => {
   // check that it's calling the right nock method
   await containerRegistry.createNetwork(providerConnectionInfo, { Name: 'myNetwork' });
 });
+
+test('setupConnectionAPI with errors', async () => {
+  // create a stream that we return to nock
+  const stream = new PassThrough();
+  // need to reply with a stream
+  nock('http://localhost').get('/events').reply(200, stream);
+
+  const internalContainerProvider: InternalContainerProvider = {
+    name: 'podman',
+    id: 'podman1',
+    connection: {
+      type: 'podman',
+      name: 'podman',
+      endpoint: {
+        socketPath: 'http://localhost',
+      },
+      status: () => 'started',
+    },
+  };
+
+  const providerConnectionInfo: podmanDesktopAPI.ContainerProviderConnection = {
+    name: 'podman',
+    type: 'podman',
+    endpoint: {
+      socketPath: '/endpoint1.sock',
+    },
+    status: () => 'started',
+  };
+
+  // check that api is being added
+  expect(internalContainerProvider.api).toBeUndefined();
+  expect(internalContainerProvider.libpodApi).toBeUndefined();
+  containerRegistry.setupConnectionAPI(internalContainerProvider, providerConnectionInfo);
+
+  // change delay of setRetryDelayEvents to be 200ms
+  containerRegistry.setRetryDelayEvents(200);
+
+  // wait 0.5s
+  await new Promise(resolve => setTimeout(resolve, 500));
+  expect(internalContainerProvider.api).toBeDefined();
+
+  // ok now send an error
+
+  // and send an error in the stream
+  stream.emit('error', new Error('my error'));
+  // close the stream
+  stream.end();
+
+  // we should not have the api anymore
+  expect(internalContainerProvider.api).toBeUndefined();
+
+  // and it should try to reconnect to the nock
+
+  // wait 0.5s
+  await new Promise(resolve => setTimeout(resolve, 500));
+
+  // mock again /events
+  const stream2 = new PassThrough();
+  nock('http://localhost').get('/events').reply(200, stream2);
+
+  // emit a container start event, we should proceed it as expected
+  const fakeId = '123456';
+  stream2.write(
+    JSON.stringify({
+      status: 'start',
+      Type: 'container',
+      id: fakeId,
+    }),
+  );
+  // check apiSender if we have a message 'container-started-event' with the right id
+  await new Promise(resolve => setTimeout(resolve, 1000));
+  expect(internalContainerProvider.api).toBeDefined();
+
+  // last call should be with the 'container-started-event' message
+  const allCalls = vi.mocked(apiSender.send).mock.calls;
+  expect(allCalls).toBeDefined();
+  const lastCall = allCalls[allCalls.length - 1];
+  expect(lastCall).toStrictEqual(['container-started-event', fakeId]);
+
+  stream2.end();
+
+  // it should have reconnect to the stream now and add again the api object
+  expect(internalContainerProvider.api).toBeDefined();
+});
diff --git a/packages/main/src/plugin/container-registry.ts b/packages/main/src/plugin/container-registry.ts
@@ -90,6 +90,9 @@ export class ContainerProviderRegistry {
   private readonly _onEvent = new Emitter<JSONEvent>();
   readonly onEvent: Event<JSONEvent> = this._onEvent.event;
 
+  // delay in ms before retrying to connect to the provider when /events connection fails
+  protected retryDelayEvents: number = 5000;
+
   private envfileParser = new EnvfileParser();
 
   constructor(
@@ -111,10 +114,13 @@ export class ContainerProviderRegistry {
   protected streamsPerContainerId: Map<string, NodeJS.ReadWriteStream> = new Map();
   protected streamsOutputPerContainerId: Map<string, Buffer[]> = new Map();
 
-  handleEvents(api: Dockerode) {
+  handleEvents(api: Dockerode, errorCallback: (error: Error) => void) {
+    let nbEvents = 0;
+    const startDate = performance.now();
     const eventEmitter = new EventEmitter();
 
     eventEmitter.on('event', (jsonEvent: JSONEvent) => {
+      nbEvents++;
       console.log('event is', jsonEvent);
       this._onEvent.fire(jsonEvent);
       if (jsonEvent.status === 'stop' && jsonEvent?.Type === 'container') {
@@ -163,7 +169,21 @@ export class ContainerProviderRegistry {
     api.getEvents((err, stream) => {
       if (err) {
         console.log('error is', err);
+        errorCallback(new Error('Error in handling events', err));
       }
+
+      stream?.on('error', error => {
+        console.error('/event stream received an error.', error);
+        // log why it failed and after how many ms connection dropped
+        this.telemetryService.track('handleContainerEventsFailure', {
+          nbEvents,
+          failureAfter: performance.now() - startDate,
+          error,
+        });
+        // notify the error (do not throw as we're inside handlers/callbacks)
+        errorCallback(new Error('Error in handling events', error));
+      });
+
       const pipeline = stream?.pipe(StreamValues.withParser());
       pipeline?.on('error', error => {
         console.error('Error while parsing events', error);
@@ -210,11 +230,33 @@ export class ContainerProviderRegistry {
     internalProvider: InternalContainerProvider,
     containerProviderConnection: containerDesktopAPI.ContainerProviderConnection,
   ) {
+    // abort if connection is stopped
+    if (containerProviderConnection.status() === 'stopped') {
+      console.log('Aborting reconnect due to error as connection is now stopped');
+      return;
+    }
+
     internalProvider.api = new Dockerode({ socketPath: containerProviderConnection.endpoint.socketPath });
     if (containerProviderConnection.type === 'podman') {
       internalProvider.libpodApi = internalProvider.api as unknown as LibPod;
     }
-    this.handleEvents(internalProvider.api);
+
+    // in case of errors reported during handling events like the connection is aborted, etc.
+    // we need to reconnect the provider
+    const errorHandler = (error: Error) => {
+      console.warn('Error when handling events', error, 'Will reconnect in 5s', error);
+      internalProvider.api = undefined;
+      internalProvider.libpodApi = undefined;
+
+      // ok we had some errors so we need to reconnect the provider
+      // delay the reconnection to avoid too many reconnections
+      // retry in 5 seconds
+      setTimeout(() => {
+        this.setupConnectionAPI(internalProvider, containerProviderConnection);
+      }, this.retryDelayEvents);
+    };
+
+    this.handleEvents(internalProvider.api, errorHandler);
     this.apiSender.send('provider-change', {});
   }