feat!: reconnect IPC and recover subscriptions after a backend restart

.github/actions/run-browserstack-maestro/action.yml

@@ -35,6 +35,15 @@ inputs:
       connection resetting during launchApp), never on a real test failure.
     required: false
     default: "3"
+  artifact_name:
+    description: >-
+      Name of the workflow artifact that receives failure diagnostics
+      (device logs, screenshots, session JSON). Defaults to
+      browserstack-diagnostics-<platform>. Artifact names must be unique
+      per workflow run, so override this if one workflow calls this action
+      more than once per platform.
+    required: false
+    default: ""
 
 runs:
   using: composite
@@ -73,6 +82,7 @@ runs:
         PLATFORM: ${{ inputs.platform }}
         TIMEOUT: ${{ fromJson(inputs.timeout) }}
         MAX_ATTEMPTS: ${{ fromJson(inputs.max_attempts) }}
+        DIAG_DIR: ${{ runner.temp }}/browserstack-diagnostics
       shell: bash
       run: | #shell
         set -o pipefail
@@ -184,6 +194,61 @@ runs:
           done
         }
 
+        # Saves what BrowserStack already recorded (deviceLogs: true above)
+        # for each failed session into DIAG_DIR — device logs, screenshots,
+        # session JSON — for the upload step, and prints the app-relevant
+        # device-log lines so the stalled/failed spec is named in the job log.
+        # Best-effort by design: always returns 0 so a fetch error here can
+        # never mask the real failure.
+        collect_diagnostics() {
+          local build_id=$1 attempt=$2
+          mkdir -p "$DIAG_DIR" || return 0
+          printf '%s\n' "$BUILD_RESPONSE" > "$DIAG_DIR/attempt${attempt}-build.json" || true
+          while IFS=$'\t' read -r device session_id; do
+            [ -n "$session_id" ] || continue
+            local slug prefix detail
+            slug=$(printf '%s' "$device" | tr -cs 'A-Za-z0-9._-' '-')
+            prefix="$DIAG_DIR/attempt${attempt}-${slug}"
+            detail=$(curl --show-error -s -u "$AUTH" "$API/builds/$build_id/sessions/$session_id") || continue
+            printf '%s\n' "$detail" > "${prefix}-session.json" || true
+            while IFS=$'\t' read -r tc_name tc_status device_log screenshots video; do
+              [ -n "$tc_name" ] || continue
+              local tc_slug base
+              tc_slug=$(printf '%s' "$tc_name" | tr -cs 'A-Za-z0-9._-' '-')
+              base="${prefix}-${tc_slug}"
+              if [ -n "$device_log" ]; then
+                curl --show-error -s -u "$AUTH" -o "${base}-device.log" "$device_log" || true
+                if [ -s "${base}-device.log" ]; then
+                  echo "::group::Device log (app lines) — $device / $tc_name ($tc_status)"
+                  matches=$({ grep -E 'ReactNativeJS|\[e2e\]|ComapeoCore|Comapeo:NodeJS' "${base}-device.log" || true; } | tail -n 200)
+                  if [ -n "$matches" ]; then
+                    printf '%s\n' "$matches"
+                  else
+                    echo "(no app-tagged lines — expected on iOS, where console output is not persisted to the device log; use the screenshots)"
+                  fi
+                  echo "::endgroup::"
+                fi
+              fi
+              if [ -n "$screenshots" ]; then
+                curl --show-error -sL -u "$AUTH" -o "${base}-screenshots.zip" "$screenshots" || true
+                if [ -s "${base}-screenshots.zip" ] \
+                  && unzip -o -q -d "${base}-screenshots" "${base}-screenshots.zip"; then
+                  rm -f "${base}-screenshots.zip"
+                fi
+              fi
+              if [ -n "$video" ]; then
+                echo "Video for $device / $tc_name: $video"
+              fi
+            done < <(printf '%s\n' "$detail" \
+              | jq -r '.testcases.data[]?.testcases[]? | select(.status != "passed")
+                  | [.name, .status, .device_log // "", .screenshots // "", .video // ""] | @tsv')
+          done < <(printf '%s\n' "$BUILD_RESPONSE" \
+            | jq -r '.devices[]? as $d | $d.sessions[]? | select(.status != "passed")
+                | [(($d.device // $d.os // "device") + "-" + ($d.os_version // "")), .id] | @tsv')
+          echo "Diagnostics saved for upload as a workflow artifact."
+          return 0
+        }
+
         # Returns 0 only if there is at least one failed session and every
         # failed session is infra-class: either its structured session
         # `error.message` matches SESSION_ERR_RE (the session never started)
@@ -243,12 +308,23 @@ runs:
             continue
           fi
 
+          collect_diagnostics "$BUILD_ID" "$attempt" || true
           echo "Tests failed (status: $STATUS)"
           exit 1
         done
 
         exit 1
 
+    # Runs only after the run step has already failed, so uploading (or a
+    # failure to upload) cannot change the action's outcome or exit code.
+    - name: Upload BrowserStack diagnostics
+      if: failure()
+      uses: actions/upload-artifact@v7
+      with:
+        name: ${{ inputs.artifact_name != '' && inputs.artifact_name || format('browserstack-diagnostics-{0}', inputs.platform) }}
+        path: ${{ runner.temp }}/browserstack-diagnostics
+        if-no-files-found: ignore
+
     - name: Stop BrowserStack build on cancel
       if: cancelled() && steps.run.outputs.build_id != ''
       env:

.github/workflows/android-tests.yml

@@ -17,7 +17,7 @@ permissions:
   contents: read
 
 env:
-  NODEJS_MOBILE_VERSION: v18.20.4
+  NODEJS_MOBILE_VERSION: v24.19.0-0
   # NDK the generated example app builds against. Sourced from React Native's
   # node_modules/react-native/gradle/libs.versions.toml (Expo's default); keep
   # in sync on RN bumps. We install it explicitly (with retry) so Gradle never

.github/workflows/e2e-reusable.yml

@@ -41,7 +41,7 @@ on:
         required: true
 
 env:
-  NODEJS_MOBILE_VERSION: v18.20.4
+  NODEJS_MOBILE_VERSION: v24.19.0-0
 
 jobs:
   # Decide whether the expensive build + paid BrowserStack device jobs run.

CONTRIBUTING.md

@@ -22,7 +22,11 @@ npm run setup      # fetch nodejs-mobile, build the backend, install the test ap
 `npm run setup` runs, in order:
 
 - `download:nodejs-mobile` — pulls `NodeMobile.xcframework` (iOS) and `libnode.so`
-  per ABI plus headers (Android) into place; these are not committed.
+  per ABI plus headers (Android) into place; these are not committed. It fetches
+  the `lite` runtime flavour, which drops ICU, the inspector, `node:sqlite` and
+  TypeScript type-stripping — none of which we use. `NODEJS_MOBILE_FLAVOR=full`
+  fetches the full one; the two ship identical headers, so addon prebuilds work
+  against either.
 - `backend:build` — bundles the Node.js backend (`backend/`) that gets embedded in
   the app. `npm install` alone does **not** build it.
 - installs dependencies for the two test apps (`apps/integration`, `apps/e2e`).
@@ -40,11 +44,13 @@ debug ID in the bundle, the consuming app uploads the maps with
 `comapeo-rn-upload-sourcemaps`, and Sentry matches them by that ID.
 
 The backend deliberately does **not** run with `--enable-source-maps` in any
-variant. nodejs-mobile pins Node 18, whose `findSourceMap()` re-parses the whole
-map on every `Error.stack` format — roughly 320 ms and 250–470 MB of garbage per
-error for our 19 MB map, enough to wedge the event loop for tens of seconds on a
-low-end device. For a stack you have in a terminal rather than in Sentry,
-`comapeo-rn-symbolicate` remaps it offline from the shipped maps.
+variant. Measured on the Node 18 nodejs-mobile used to pin, `findSourceMap()`
+re-parsed the whole map on every `Error.stack` format — roughly 320 ms and
+250–470 MB of garbage per error for our 19 MB map, enough to wedge the event
+loop for tens of seconds on a low-end device. Node has since reworked its
+source-map cache; the flag stays off until that's re-measured on device. For a
+stack you have in a terminal rather than in Sentry, `comapeo-rn-symbolicate`
+remaps it offline from the shipped maps.
 
 ## Repository layout
 

README.md

@@ -364,10 +364,10 @@ place of the flags.
 
 The maps live in sibling `nodejs-sourcemaps/` directories (not under the bundled
 `nodejs-project/` assets), so they are **not** shipped inside your APK/IPA. The
-backend runs without Node's `--enable-source-maps` in every variant — on the
-Node 18 that nodejs-mobile pins, that flag re-parses the entire map on every
-error stack and can wedge the event loop for tens of seconds on a low-end
-device. To remap a stack you have in a terminal rather than in Sentry:
+backend runs without Node's `--enable-source-maps` in every variant — measured
+on Node 18, that flag re-parsed the entire map on every error stack and could
+wedge the event loop for tens of seconds on a low-end device. To remap a stack
+you have in a terminal rather than in Sentry:
 
 ```sh
 adb logcat -d | npx comapeo-rn-symbolicate

android/CMakeLists.txt

@@ -33,6 +33,11 @@ add_library(${CMAKE_PROJECT_NAME} SHARED
 include_directories(libnode/include/node/)
 include_directories(src/main/cpp)
 
+# Node 24's v8config.h #errors below C++20; the NDK's clang defaults to gnu++17.
+set_target_properties(${CMAKE_PROJECT_NAME} PROPERTIES
+    CXX_STANDARD 20
+    CXX_STANDARD_REQUIRED ON)
+
 add_library( libnode
              SHARED
              IMPORTED )

android/src/androidTest/java/com/comapeo/core/NodeJSIPCTest.kt

@@ -61,16 +61,21 @@ class NodeJSIPCTest {
      * To match, we bind a [LocalSocket] to the filesystem address and pass
      * its file descriptor to [LocalServerSocket].
      */
-    private fun startMockServer(onConnection: (DataInputStream, DataOutputStream) -> Unit) {
+    private fun bindServer(): LocalServerSocket {
         val bindSocket = LocalSocket(LocalSocket.SOCKET_STREAM)
         val address = LocalSocketAddress(socketFile.absolutePath, LocalSocketAddress.Namespace.FILESYSTEM)
         bindSocket.bind(address)
         boundSocket = bindSocket
         serverSocket = LocalServerSocket(bindSocket.fileDescriptor)
+        return serverSocket!!
+    }
+
+    private fun startMockServer(onConnection: (DataInputStream, DataOutputStream) -> Unit) {
+        val server = bindServer()
 
         Thread {
             try {
-                val client = serverSocket!!.accept()
+                val client = server.accept()
                 val input = DataInputStream(client.inputStream)
                 val output = DataOutputStream(client.outputStream)
                 onConnection(input, output)
@@ -351,4 +356,127 @@ class NodeJSIPCTest {
         // The IPC should handle the server disconnect without crashing
         ipc.disconnect()
     }
+
+    /**
+     * With reconnectOnDrop enabled, an unexpected server-side close followed by
+     * the server accepting again must converge back to a working connection
+     * without any sendMessage()/connect() nudge — the low-memory-kill recovery
+     * path where the FGS process restarts a few seconds later.
+     */
+    @Test
+    fun reconnectsAfterUnexpectedServerDrop() {
+        val server = bindServer()
+        val firstAccepted = CountDownLatch(1)
+        val received = CountDownLatch(1)
+
+        Thread {
+            try {
+                val first = server.accept()
+                firstAccepted.countDown()
+                Thread.sleep(300)
+                first.close() // unexpected drop
+                val second = server.accept()
+                Thread.sleep(300) // let the reconnected client's receive loop attach
+                writeFramedMessage(
+                    DataOutputStream(second.outputStream),
+                    """{"type":"after-reconnect"}""",
+                )
+                Thread.sleep(5000)
+            } catch (e: IOException) {
+                // Server closed, expected during teardown
+            }
+        }.start()
+
+        val ipc = NodeJSIPC(socketFile, reconnectOnDrop = true) { msg ->
+            receivedMessages.add(msg)
+            received.countDown()
+        }
+        try {
+            assertTrue("Should connect within 10s", firstAccepted.await(10, TimeUnit.SECONDS))
+            assertTrue(
+                "Should auto-reconnect and receive within 15s",
+                received.await(15, TimeUnit.SECONDS)
+            )
+            assertEquals("""{"type":"after-reconnect"}""", receivedMessages[0])
+        } finally {
+            ipc.close()
+        }
+    }
+
+    @Test
+    fun closeSuppressesReconnect() {
+        val server = bindServer()
+        val acceptCount = java.util.concurrent.atomic.AtomicInteger(0)
+        // Retain every accepted socket: an unreferenced LocalSocket can be
+        // GC-finalized (closed) mid-test, dropping the connection and causing
+        // a spurious reconnect before close() is even called.
+        val acceptedSockets = CopyOnWriteArrayList<LocalSocket>()
+
+        Thread {
+            try {
+                while (true) {
+                    acceptedSockets.add(server.accept())
+                    acceptCount.incrementAndGet()
+                }
+            } catch (e: IOException) {
+                // Server closed, expected during teardown
+            }
+        }.start()
+
+        val ipc = NodeJSIPC(socketFile, reconnectOnDrop = true) { msg ->
+            receivedMessages.add(msg)
+        }
+        val deadline = System.currentTimeMillis() + 10_000
+        while (acceptCount.get() == 0 && System.currentTimeMillis() < deadline) {
+            Thread.sleep(50)
+        }
+        assertEquals("Should connect once", 1, acceptCount.get())
+        Thread.sleep(200)
+
+        ipc.close()
+
+        // Longer than several backoff steps (immediate + 250 + 500 + 1000ms); a
+        // post-close reconnect would show up as a second accept.
+        Thread.sleep(3000)
+        assertEquals("close() must not trigger reconnect attempts", 1, acceptCount.get())
+        acceptedSockets.forEach { try { it.close() } catch (_: IOException) {} }
+    }
+
+    @Test
+    fun doesNotReconnectByDefaultAfterUnexpectedDrop() {
+        val server = bindServer()
+        val acceptCount = java.util.concurrent.atomic.AtomicInteger(0)
+
+        Thread {
+            try {
+                while (true) {
+                    val client = server.accept()
+                    acceptCount.incrementAndGet()
+                    Thread.sleep(300)
+                    client.close() // unexpected drop
+                }
+            } catch (e: IOException) {
+                // Server closed, expected during teardown
+            }
+        }.start()
+
+        val ipc = NodeJSIPC(socketFile) { msg -> receivedMessages.add(msg) }
+        try {
+            val deadline = System.currentTimeMillis() + 10_000
+            while (acceptCount.get() == 0 && System.currentTimeMillis() < deadline) {
+                Thread.sleep(50)
+            }
+            assertEquals("Should connect once", 1, acceptCount.get())
+
+            // Ample time for an (unwanted) reconnect after the drop at +300ms.
+            Thread.sleep(3000)
+            assertEquals(
+                "Default (reconnectOnDrop=false) must not reconnect",
+                1,
+                acceptCount.get()
+            )
+        } finally {
+            ipc.close()
+        }
+    }
 }

android/src/main/cpp/jni-bridge.cpp

@@ -97,6 +97,16 @@ class NodeJSService : public JavaClass<NodeJSService> {
         log("initialize: %s", nativeDataDir.c_str());
     }
 
+    /// node reads TMPDIR and NODE_COMPILE_CACHE while the Environment is
+    /// created, so assigning `process.env` from JS is too late — callers must
+    /// set them here, before `startNodeWithArguments`.
+    static void setEnv(alias_ref<JClass>, alias_ref<jstring> name, alias_ref<jstring> value) {
+        const auto nativeName = name->toStdString();
+        const auto nativeValue = value->toStdString();
+        setenv(nativeName.c_str(), nativeValue.c_str(), 1);
+        log("setEnv: %s=%s", nativeName.c_str(), nativeValue.c_str());
+    }
+
     static jint startNodeWithArguments(alias_ref<JClass>,
                                        alias_ref<JArrayClass<jstring>> arguments) {
         log("Starting NodeJS with arguments.");
@@ -141,6 +151,8 @@ class NodeJSService : public JavaClass<NodeJSService> {
         javaClassStatic()->registerNatives({
                                                    makeNativeMethod("initialize",
                                                                     NodeJSService::initialize),
+                                                   makeNativeMethod("setEnv",
+                                                                    NodeJSService::setEnv),
                                                    makeNativeMethod("startNodeWithArguments",
                                                                     NodeJSService::startNodeWithArguments),
                                            });