bug(satellite): Cleanup startingPromise in registry when the Satellit…

…e process fails to start (#596)

When the satellite process fails (local state is invalid or out of date,
invalid migrations, etc...), if the app cleans up the state and retries
again (without a browser refresh), it currently will keep failing,
because the `startingPromise` is being cached and not released when the
promise rejects.

.changeset/shiny-steaks-raise.md

@@ -0,0 +1,5 @@
+---
+"electric-sql": patch
+---
+
+Cleanup startingPromise in registry when the Satellite process fails to start

clients/typescript/src/satellite/mock.ts

@@ -99,6 +99,12 @@ export class MockSatelliteProcess implements Satellite {
 }
 
 export class MockRegistry extends BaseRegistry {
+  private shouldFailToStart = false
+
+  setShouldFailToStart(shouldFail: boolean): void {
+    this.shouldFailToStart = shouldFail
+  }
+
   async startProcess(
     dbName: DbName,
     _dbDescription: DbSchema<any>,
@@ -109,6 +115,10 @@ export class MockRegistry extends BaseRegistry {
     config: ElectricConfig,
     overrides?: SatelliteOverrides
   ): Promise<Satellite> {
+    if (this.shouldFailToStart) {
+      throw new Error('Failed to start satellite process')
+    }
+
     const opts = { ...satelliteDefaults, ...overrides }
 
     const satellite = new MockSatelliteProcess(

clients/typescript/src/satellite/registry.ts

@@ -107,13 +107,16 @@ export abstract class BaseRegistry implements Registry {
       notifier,
       socketFactory,
       config
-    ).then((satellite) => {
-      delete startingPromises[dbName]
-
-      satellites[dbName] = satellite
+    )
+      .then((satellite) => {
+        satellites[dbName] = satellite
 
-      return satellite
-    })
+        return satellite
+      })
+      .finally(() => {
+        // Clean up the starting promise, whether it resolved or rejected.
+        delete startingPromises[dbName]
+      })
 
     startingPromises[dbName] = startingPromise
     return startingPromise

clients/typescript/test/satellite/registry.test.ts

@@ -79,6 +79,24 @@ test('ensure satellite process started works', async (t) => {
   t.true(satellite instanceof MockSatelliteProcess)
 })
 
+test('can stop failed process', async (t) => {
+  const mockRegistry = new MockRegistry()
+
+  // Mock the satellite process to fail.
+  mockRegistry.setShouldFailToStart(true)
+
+  await t.throwsAsync(mockRegistry.ensureStarted(...args))
+
+  // This should not throw. This tests that failed processes are properly cleaned up.
+  await mockRegistry.stop(dbName)
+
+  // Next run a successful process to make sure that works.
+  mockRegistry.setShouldFailToStart(false)
+
+  const satellite = await mockRegistry.ensureStarted(...args)
+  t.true(satellite instanceof MockSatelliteProcess)
+})
+
 test('ensure starting multiple satellite processes works', async (t) => {
   const mockRegistry = new MockRegistry()
   const s1 = await mockRegistry.ensureStarted(