fix(ci): stop the detached telemetry flush too, re-signal stragglers on a monotonic clock, and report HTTP E2E failures by route (#8706)

* fix(e2e): re-signal session stragglers on a monotonic clock and report HTTP E2E failures once, by route

- stop-session.sh times its grace period from /proc/uptime, which is monotonic and
  free of the locale's decimal separator, re-signals every 0.1s so a process that
  joins the session after the first signal is stopped too, and explains why zombies
  don't count
- version compare: a request whose body times out names its route like a request
  that never answered
- desktop inbox: callers that join an executor pull in flight share it, so a failed
  pull is logged once

* fix(e2e): log a failed executor pull with its stack

* fix(ci): also stop the detached telemetry flush an HTTP end-to-end app leaves behind

`next dev` spawns its telemetry flush detached, in a session of its own, even with
telemetry disabled. That process outlives the app's session and writes .next/dev/trace
after stop-session.sh returned, so the next app's `rm -rf .next/dev` could fail with
"Directory not empty". Each app now starts under an E2E_APP tag that every process it
spawns inherits, and stop-session.sh stops and waits for tagged processes as well as the
session's members.

Author: waleedlatif1

.github/scripts/stop-session.sh

@@ -1,62 +1,84 @@
 #!/usr/bin/env bash
-# Stops every process in a session started with `setsid`, and returns only once none is left.
+# Stops every process an app started, and returns only once none is left.
 #
-# Usage: stop-session.sh <session-leader-pid>
+# Usage: stop-session.sh <session-leader-pid> <app-tag>
 #
-# `next dev` runs its server and workers as child processes. Waiting on `next dev` alone returns
-# while those can still be running, and the next app in the job reuses the same .next directory.
-# Every member of the session is signalled, including one that moved to its own process group.
-# The leader stays a zombie until the shell that started it waits on it, so zombies don't count.
+# Start the app as `E2E_APP=<app-tag> setsid <command> &`. Its processes are the members of the
+# leader's session, plus any process that inherited `E2E_APP=<app-tag>` but left the session.
+# `next dev` needs both: its server and workers stay in the session, while its telemetry flush is
+# spawned detached, in a session of its own, and writes .next/dev/trace after `next dev` exits.
+# The next app in the job wipes that directory, so it must not start while any of them runs.
+#
+# Every member is signalled, including one that moved to its own process group, and one that
+# started after the first signal.
+#
+# Zombies don't count as running: they have exited and only await reaping. The leader is one
+# until the shell that started it waits on it, which that shell does after this returns.
 set -u
 
 leader=$1
+tag=$2
 grace_seconds=10
 
-session_pids() {
-  ps -s "$leader" -o pid=,stat= 2>/dev/null | awk '$2 !~ /^Z/ { print $1 }'
+app_pids() {
+  {
+    ps -s "$leader" -o pid=,stat= 2>/dev/null | awk '$2 !~ /^Z/ { print $1 }'
+    grep -lzx "E2E_APP=$tag" /proc/[0-9]*/environ 2>/dev/null | cut -d/ -f3
+  } | sort -u
 }
 
-signal_session() {
+signal_app() {
   local pids
-  pids=$(session_pids)
+  pids=$(app_pids)
   [ -z "$pids" ] || kill "-$1" $pids 2>/dev/null || true
 }
 
 running() {
-  [ -n "$(session_pids)" ]
+  [ -n "$(app_pids)" ]
 }
 
 leader_running() {
   ps -p "$leader" -o stat= 2>/dev/null | grep -qv '^Z'
 }
 
-now_us() {
-  echo "${EPOCHREALTIME/./}"
+list_app() {
+  local pids
+  pids=$(app_pids | paste -sd, -)
+  [ -z "$pids" ] || ps -p "$pids" -o pid,sid,stat,etimes,args 2>/dev/null | cut -c1-200 || true
+}
+
+# Centiseconds since boot: monotonic, and independent of the locale's decimal separator.
+now_cs() {
+  local uptime
+  read -r uptime _ < /proc/uptime
+  echo "${uptime/./}"
 }
 
-# Waits while the given check holds, until the shared deadline. Succeeds once it stops holding.
-wait_while() {
-  while (($(now_us) < deadline)); do
+# Re-sends the signal every 0.1s while the check holds, until the shared deadline.
+# Succeeds once the check stops holding.
+signal_while() {
+  local signal=$1
+  shift
+  while ((10#$(now_cs) < deadline)); do
     "$@" || return 0
+    signal_app "$signal"
     sleep 0.1
   done
   ! "$@"
 }
 
-deadline=$(($(now_us) + grace_seconds * 1000000))
-signal_session TERM
-wait_while leader_running
+deadline=$((10#$(now_cs) + grace_seconds * 100))
+signal_while TERM leader_running
 if ! leader_running && running; then
-  echo "Processes from session $leader outlived its leader:"
-  ps -s "$leader" -o pid,stat,etimes,args 2>/dev/null | cut -c1-200 || true
+  echo "Processes from app $tag outlived its leader:"
+  list_app
 fi
-wait_while running && exit 0
+signal_while TERM running && exit 0
 
-echo "::warning::Processes from session $leader were still running ${grace_seconds}s after SIGTERM:"
-ps -s "$leader" -o pid,stat,etimes,args 2>/dev/null | cut -c1-200 || true
-deadline=$(($(now_us) + grace_seconds * 1000000))
-signal_session KILL
-wait_while running && exit 0
+echo "::warning::Processes from app $tag were still running ${grace_seconds}s after SIGTERM:"
+list_app
+deadline=$((10#$(now_cs) + grace_seconds * 100))
+signal_while KILL running && exit 0
 
-echo "::error::Processes from session $leader survived SIGKILL for ${grace_seconds}s."
+echo "::error::Processes from app $tag survived SIGKILL for ${grace_seconds}s."
 exit 1

.github/workflows/test-build.yml

@@ -211,8 +211,10 @@ jobs:
       # same lifecycle. It starts from an empty Turbopack dev cache: a cache written under
       # other NEXT_PUBLIC_* values, by a server that `next dev` SIGKILLs 100ms after
       # SIGTERM, can panic Turbopack or wedge a route compile on restore. It runs in its
-      # own session, and stop-session.sh returns only once all of it has exited, so no app
-      # starts beside one still writing .next/dev.
+      # own session under an E2E_APP tag, and stop-session.sh returns only once every
+      # process in that session or carrying that tag has exited (Next's telemetry flush
+      # runs detached and still writes .next/dev), so no app starts beside one still
+      # writing .next/dev.
       - name: Verify SCIM, administration and workflow comparisons over real HTTP
         working-directory: apps/sim
         env:
@@ -240,11 +242,11 @@ jobs:
           server_log="$report_dir/scim-next.log"
           mkdir -p "$report_dir"
           rm -rf .next/dev
-          setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3017 > "$server_log" 2>&1 &
+          E2E_APP=scim setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3017 > "$server_log" 2>&1 &
           server_pid=$!
           finish() {
             status=$?
-            bash ../../.github/scripts/stop-session.sh "$server_pid" || status=1
+            bash ../../.github/scripts/stop-session.sh "$server_pid" scim || status=1
             wait "$server_pid" 2>/dev/null || true
             awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/scim-http-status.log"
             if [ "$status" -ne 0 ]; then
@@ -293,11 +295,11 @@ jobs:
           server_log="$report_dir/cli-next.log"
           mkdir -p "$report_dir"
           rm -rf .next/dev
-          setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
+          E2E_APP=cli setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
           server_pid=$!
           finish() {
             status=$?
-            bash ../../.github/scripts/stop-session.sh "$server_pid" || status=1
+            bash ../../.github/scripts/stop-session.sh "$server_pid" cli || status=1
             wait "$server_pid" 2>/dev/null || true
             if [ "$status" -ne 0 ]; then
               tail -n 200 "$server_log"
@@ -344,11 +346,11 @@ jobs:
           server_log="$report_dir/stop-after-next.log"
           mkdir -p "$report_dir"
           rm -rf .next/dev
-          setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
+          E2E_APP=stop-after setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3018 > "$server_log" 2>&1 &
           server_pid=$!
           finish() {
             status=$?
-            bash ../../.github/scripts/stop-session.sh "$server_pid" || status=1
+            bash ../../.github/scripts/stop-session.sh "$server_pid" stop-after || status=1
             wait "$server_pid" 2>/dev/null || true
             awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/stop-after-http-status.log"
             if [ "$status" -ne 0 ]; then
@@ -395,11 +397,11 @@ jobs:
           server_log="$report_dir/desktop-inbox-next.log"
           mkdir -p "$report_dir"
           rm -rf .next/dev
-          setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3019 > "$server_log" 2>&1 &
+          E2E_APP=desktop-inbox setsid node ../../node_modules/next/dist/bin/next dev --hostname 127.0.0.1 --port 3019 > "$server_log" 2>&1 &
           server_pid=$!
           finish() {
             status=$?
-            bash ../../.github/scripts/stop-session.sh "$server_pid" || status=1
+            bash ../../.github/scripts/stop-session.sh "$server_pid" desktop-inbox || status=1
             wait "$server_pid" 2>/dev/null || true
             awk '/^ (GET|POST|PUT|PATCH|DELETE|HEAD) \/api\// { print }' "$server_log" > "$report_dir/desktop-inbox-http-status.log"
             if [ "$status" -ne 0 ]; then

apps/sim/scripts/test-desktop-inbox-e2e.ts

@@ -358,22 +358,17 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
       inFlight = null
     }
   }
+  /** Callers that join a pull in flight share it, so its failure is logged once, here. */
   const pull = () => {
     if (inFlight) {
       rerun = true
-      return inFlight
+      return
     }
-    inFlight = drain()
-    return inFlight
+    inFlight = drain().catch((error) => logger.error('executor pull failed', error))
   }
-  const offDoorbell = doorbell.onEvent(
-    () => void pull().catch((error) => logger.error('pull failed', error))
-  )
-  const timer = setInterval(
-    () => void pull().catch((error) => logger.error('reconcile failed', error)),
-    RECONCILE_MS
-  )
-  void pull().catch((error) => logger.error('pull failed', error))
+  const offDoorbell = doorbell.onEvent(pull)
+  const timer = setInterval(pull, RECONCILE_MS)
+  pull()
   return {
     ran,
     completed,
@@ -385,7 +380,7 @@ function startExecutor(desktop: Desktop, doorbell: ReturnType<typeof openDoorbel
       stopped = true
       offDoorbell()
       clearInterval(timer)
-      await inFlight?.catch(() => {})
+      await inFlight
     },
   }
 }

apps/sim/scripts/test-workflow-version-compare-e2e.ts

@@ -356,10 +356,18 @@ async function get(path: string, auth: { key?: string; session?: string } = {},
   assert.match(response.headers.get('content-type') ?? '', /application\/json/)
   if (path.startsWith('/api/v2/'))
     assert.equal(response.headers.get('cache-control'), 'private, no-store')
-  const text = await readResponseTextWithLimit(response, {
-    maxBytes: MAX_RESPONSE_BYTES,
-    label: 'Version comparison E2E response',
-  })
+  let text: string
+  try {
+    text = await readResponseTextWithLimit(response, {
+      maxBytes: MAX_RESPONSE_BYTES,
+      label: 'Version comparison E2E response',
+    })
+  } catch (error) {
+    /** `get` passes no signal of its own, so an abort here is the request timeout. */
+    if (!(error instanceof DOMException && ['TimeoutError', 'AbortError'].includes(error.name)))
+      throw error
+    throw new Error(`GET ${path}: response body not complete within ${REQUEST_TIMEOUT_MS / 1000}s`)
+  }
   return record(JSON.parse(text))
 }