Skip to content

fix: add retry logic for WebSocket and SSE startup failures - #119

Merged
guarzo merged 6 commits into
mainfrom
fix/startup-retry-resilience
Apr 19, 2026
Merged

fix: add retry logic for WebSocket and SSE startup failures#119
guarzo merged 6 commits into
mainfrom
fix/startup-retry-resilience

Conversation

@guarzo

@guarzo guarzo commented Apr 19, 2026

Copy link
Copy Markdown
Owner

When upstream services are temporarily unreachable during app startup, both WebSocket and SSE clients would fail their initial connection with no recovery path — leaving the app in a permanently broken state until manually restarted.

WebSocket: Replace fire-and-forget Task with retry loop using exponential backoff (5s–60s) in the Killmail Supervisor GenServer.

SSE: Retry failed map initialization up to 5 times with exponential backoff (10s–60s) via TaskSupervisor, re-attempting initialize_and_start_for_map for each failed map.

Summary by CodeRabbit

  • New Features

    • Automatic connection recovery for WebSocket clients with scheduled retries, attempt tracking, and exponential backoff.
    • Per-map retry subsystem for map initialization that retries failed items individually with capped, increasing delays.
  • Improvements

    • Clearer logging for retry scheduling, attempts, successes, skips, and eventual give-up to improve observability and resilience.
  • Chores

    • Release version bumped to 6.1.4.

When upstream services are temporarily unreachable during app startup,
both WebSocket and SSE clients would fail their initial connection with
no recovery path — leaving the app in a permanently broken state until
manually restarted.

WebSocket: Replace fire-and-forget Task with retry loop using
exponential backoff (5s–60s) in the Killmail Supervisor GenServer.

SSE: Retry failed map initialization up to 5 times with exponential
backoff (10s–60s) via TaskSupervisor, re-attempting
initialize_and_start_for_map for each failed map.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Apr 19, 2026

Copy link
Copy Markdown
Contributor

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

  • @coderabbitai resume to resume automatic reviews.
  • @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

  • ▶️ Resume reviews
  • 🔍 Trigger review

Walkthrough

Killmail supervisor now starts the WebSocket client via a supervised child and implements tracked exponential-backoff retries. SSE supervisor separates successful and failed map inits, starts clients for successes, and schedules bounded exponential-backoff retries for failed map initializations.

Changes

Cohort / File(s) Summary
Killmail WebSocket Supervisor
lib/wanderer_notifier/domains/killmail/supervisor.ex
Replaced Task.Supervisor spawn with start_websocket_client/0 using Supervisor.start_child/2. On failure logs retry_in_ms, stores :ws_retry_attempts, schedules :retry_websocket, added handle_info(:retry_websocket, ...), calculate_retry_delay/1, and schedule_websocket_retry/2. Treats {:error, {:already_started, pid}} as success and clears attempts.
SSE Map Initialization Supervisor
lib/wanderer_notifier/map/sse_supervisor.ex
run_parallel_init/1 now returns {successful_maps, failed_maps}; initialize_maps/0 starts SSE clients only for successes and schedules retries for failures. Added helpers: extract_failed_maps/1, schedule_failed_map_retries/2, retry_failed_maps/2, fetch_fresh_configs/1, attempt_map_initialization/1, and calculate_init_retry_delay/1. Implements capped exponential-backoff (@max_init_retries, @max_retry_delay) and logging for scheduling, skips, recoveries, and give-up.
Project version
mix.exs
Bumped application version from 6.1.3 to 6.1.4.

Sequence Diagram(s)

sequenceDiagram
    participant Sup as Supervisor
    participant WS as WebSocketClient
    participant Sched as Scheduler

    Sup->>WS: start_websocket_client()
    alt Success
        WS-->>Sup: {:ok, pid}
        Sup->>Sup: clear :ws_retry_attempts
    else Failure
        WS-->>Sup: {:error, reason}
        Sup->>Sup: log warning (retry_in_ms)
        Sup->>Sched: schedule :retry_websocket (delay)
        Sched-->>Sup: :retry_websocket
        Sup->>Sup: increment :ws_retry_attempts
        Sup->>WS: start_websocket_client()
        alt Success
            WS-->>Sup: {:ok, pid}
            Sup->>Sup: clear :ws_retry_attempts
        else Failure
            WS-->>Sup: {:error, reason}
            Sup->>Sched: schedule :retry_websocket (increased delay)
        end
    end
Loading
sequenceDiagram
    participant Sup as SSE Supervisor
    participant Init as MapInitialization
    participant SSE as SSEClientManager
    participant Sched as Scheduler
    participant Reg as MapRegistry

    Sup->>Init: run_parallel_init()
    Init-->>Sup: {successful_maps, failed_maps}
    Sup->>SSE: start_sse_clients(successful_maps)
    alt failed_maps not empty
        Sup->>Sched: schedule_failed_map_retries(failed_maps)
        Sched-->>Sup: :retry_failed_maps (timer)
        Sup->>Reg: fetch_fresh_configs(failed_maps)
        Reg-->>Sup: fresh_configs
        Sup->>Init: retry_failed_maps(fresh_configs)
        Init-->>Sup: {new_successful, still_failed}
        Sup->>SSE: start_sse_clients(new_successful)
        alt still_failed and attempts < max
            Sup->>Sched: schedule_failed_map_retries(still_failed)
        else exceeded attempts or removed
            Sup->>Sup: log give-up / skip retries
        end
    end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hop through logs at midnight glow,

retrying sockets when connections slow,
failed maps learn to try again,
backoff carrots stack like rain,
I cheer when clients wake and go.

🚥 Pre-merge checks | ✅ 2 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (2 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely summarizes the main changes: adding retry logic for both WebSocket and SSE startup failures, which is the primary focus of all file modifications.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/startup-retry-resilience

Comment @coderabbitai help to get the list of available commands and usage tips.

- schedule_websocket_retry returns state instead of GenServer tuple
- Remove double delay calculation in handle_info(:retry_websocket)
- Separate side effects from predicate in retry_failed_maps

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/wanderer_notifier/domains/killmail/supervisor.ex`:
- Around line 81-95: The GenServer currently starts the WebSocket linked into
its process tree via
WandererNotifier.Domains.Killmail.WebSocketClient.start_link from
handle_continue(:start_websocket) (and/or start_websocket_client), which will
crash this supervisor when the socket dies; change the startup so the WebSocket
is started unlinked or under the InternalSupervisor instead: remove any direct
start_link calls from handle_continue/2 (or start_websocket_client) and either
(a) call InternalSupervisor.start_child/1 (or the existing internal supervisor
helper) to start the WebSocket under that supervisor, or (b)
spawn/Task.start_unlinked an intermediary starter that calls
WebSocketClient.start_link so the WebSocket is not linked to this GenServer;
ensure the retry scheduling logic (schedule_websocket_retry) still runs if the
external start attempt fails.

In `@lib/wanderer_notifier/map/sse_supervisor.ex`:
- Around line 204-214: The function initialize_maps/0 should always return :ok;
after calling signal_pipeline_worker() and
start_sse_clients_staggered(successful_maps) call
schedule_failed_map_retries(failed_maps, 1) but do not return its raw
Task.Supervisor.start_child/2 result—instead pattern-match its result, log any
{:error, reason} or unexpected reply (use the module logger or existing
processLogger) and ensure initialize_maps/0 ends with :ok; apply the same change
to the other occurrence (lines ~382-395) so scheduler failures are explicitly
logged but do not change the public return value.
- Around line 398-417: The retry_failed_maps/2 function currently tries to
initialize maps from a captured batch that may include maps removed in the
meantime; before calling initialize_and_start_for_map/1 filter the incoming maps
against the current registry returned by Dependencies.map_registry() (e.g. fetch
live slugs or membership and then Enum.filter maps by &(&1.slug in live_slugs))
so you only retry maps that still exist, and then proceed with the existing
split/logging and schedule_failed_map_retries/2 behavior; update references to
MapConfig.slug, retry_failed_maps/2, initialize_and_start_for_map/1,
Dependencies.map_registry(), and schedule_failed_map_retries/2 accordingly.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 85ac804b-d6da-4032-9d18-9f91c1a3a9c9

📥 Commits

Reviewing files that changed from the base of the PR and between e6dd281 and e3d7efb.

📒 Files selected for processing (2)
  • lib/wanderer_notifier/domains/killmail/supervisor.ex
  • lib/wanderer_notifier/map/sse_supervisor.ex

Comment thread lib/wanderer_notifier/domains/killmail/supervisor.ex
Comment thread lib/wanderer_notifier/map/sse_supervisor.ex
Comment thread lib/wanderer_notifier/map/sse_supervisor.ex Outdated

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

♻️ Duplicate comments (3)
lib/wanderer_notifier/map/sse_supervisor.ex (2)

392-415: ⚠️ Potential issue | 🟠 Major

Re-fetch live map configs before retrying.

The task sleeps in Lines 393-394, but Line 404 still retries the captured %MapConfig{} list. If a map is removed or updated during that backoff window, this can recreate SSE state for a deleted map or retry against stale config.

Proposed fix
 defp retry_failed_maps(maps, attempt) do
-  Logger.info("Retrying initialization for #{length(maps)} map(s)",
+  active_maps =
+    Enum.flat_map(maps, fn %{slug: slug} ->
+      case Dependencies.map_registry().get_map(slug) do
+        {:ok, map_config} -> [map_config]
+        {:error, _} -> []
+      end
+    end)
+
+  Logger.info("Retrying initialization for #{length(active_maps)} map(s)",
     attempt: attempt,
     category: :startup
   )

-  results = Enum.map(maps, fn map -> {map, initialize_and_start_for_map(map)} end)
+  results = Enum.map(active_maps, fn map -> {map, initialize_and_start_for_map(map)} end)

   {succeeded, still_failed} =
     Enum.split_with(results, fn {_map, result} -> match?({:ok, _}, result) end)

   if succeeded != [] do
     slugs = Enum.map(succeeded, fn {map, _} -> map.slug end)
     Logger.info("Map initialization retry succeeded", map_slugs: slugs, category: :startup)
   end

   failed_maps = Enum.map(still_failed, fn {map, _} -> map end)
   schedule_failed_map_retries(failed_maps, attempt + 1)
 end
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/wanderer_notifier/map/sse_supervisor.ex` around lines 392 - 415, The
retry logic in retry_failed_maps currently uses the stale maps list captured
earlier; before calling initialize_and_start_for_map you should re-fetch current
map configs (e.g. look up each map.slug or fetch all live MapConfig entries) and
only attempt initialization for configs that still exist, skipping/logging maps
that were removed or changed; update the code in retry_failed_maps to map slugs
-> fetch fresh configs, call initialize_and_start_for_map with the fresh config,
and then pass the remaining failures into schedule_failed_map_retries (keep
function names retry_failed_maps, initialize_and_start_for_map, and
schedule_failed_map_retries for easy location).

204-213: ⚠️ Potential issue | 🟠 Major

Keep initialize_sse_clients/0 returning :ok.

Line 213 still returns the raw schedule_failed_map_retries/2 result, and Lines 392-395 still bubble up Task.Supervisor.start_child/2 on partial startup failures. That changes initialize_sse_clients/0 from its declared :ok contract to {:ok, pid} | {:error, reason} exactly when map initialization is already degraded.

Proposed fix
 defp initialize_maps do
   maps = Dependencies.map_registry().all_maps()
   Logger.info("Initializing #{length(maps)} maps from registry", category: :startup)

   {successful_maps, failed_maps} = run_parallel_init(maps)

   # Signal PipelineWorker
   signal_pipeline_worker()

   # Start SSE clients only for successfully initialized maps
   start_sse_clients_staggered(successful_maps)

   # Schedule retry for failed maps
-  schedule_failed_map_retries(failed_maps, 1)
+  schedule_failed_map_retries(failed_maps, 1)
+  :ok
 end
@@
 defp schedule_failed_map_retries(failed_maps, attempt) do
   delay = calculate_init_retry_delay(attempt)
   slugs = Enum.map(failed_maps, & &1.slug)

   Logger.info(
     "Scheduling retry #{attempt}/#{`@max_init_retries`} for #{length(failed_maps)} failed map(s) in #{delay}ms",
     map_slugs: slugs,
     category: :startup
   )

-  Task.Supervisor.start_child(WandererNotifier.TaskSupervisor, fn ->
-    Process.sleep(delay)
-    retry_failed_maps(failed_maps, attempt)
-  end)
+  case Task.Supervisor.start_child(WandererNotifier.TaskSupervisor, fn ->
+         Process.sleep(delay)
+         retry_failed_maps(failed_maps, attempt)
+       end) do
+    {:ok, _pid} ->
+      :ok
+
+    {:error, reason} ->
+      Logger.error("Failed to schedule map initialization retry",
+        map_slugs: slugs,
+        attempt: attempt,
+        reason: inspect(reason),
+        category: :startup
+      )
+
+      :ok
+  end
 end

Also applies to: 382-395

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/wanderer_notifier/map/sse_supervisor.ex` around lines 204 - 213,
initialize_sse_clients/0 must always return :ok; after calling
run_parallel_init(maps), signal_pipeline_worker(),
start_sse_clients_staggered(successful_maps) and
schedule_failed_map_retries(failed_maps, 1), ignore and do not return the raw
result of schedule_failed_map_retries/2—explicitly return :ok. In
start_sse_clients_staggered/1 (and any code paths that call
Task.Supervisor.start_child/2 around lines referenced), catch failures from
Task.Supervisor.start_child/2 (use case/try/rescue or handle_info) so partial
startup errors are logged/handled locally and do not bubble up as {:error,
reason} from initialize_sse_clients/0; ensure any Task.Supervisor.start_child
results are normalized/logged and the outer initialize_sse_clients/0 still
returns :ok.
lib/wanderer_notifier/domains/killmail/supervisor.ex (1)

81-94: ⚠️ Potential issue | 🔴 Critical

Don't start_link/0 the WebSocket directly from this GenServer.

Lines 82 and 139 still start the socket via WebSocketClient.start_link/0, which links the WebSocket process to this GenServer. Because this module never traps exits, a later socket crash will take down the Killmail supervisor itself instead of being isolated under __MODULE__.InternalSupervisor.

Use the internal supervisor (or another unlinked starter) for the WebSocket start path, then keep the retry scheduling around that call.

Run this to verify the direct start_link path and the absence of exit trapping in the current tree. Expected result: WebSocketClient.start_link is called from this module, and no trap_exit handling exists here.

#!/bin/bash
set -euo pipefail

echo "=== Killmail supervisor start/retry path ==="
sed -n '80,155p' lib/wanderer_notifier/domains/killmail/supervisor.ex

echo
echo "=== Link/trap-exit evidence ==="
rg -n 'WebSocketClient\.start_link|trap_exit|InternalSupervisor' \
  lib/wanderer_notifier/domains/killmail/supervisor.ex \
  lib/wanderer_notifier/domains/killmail/websocket_client.ex

Also applies to: 136-150

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/wanderer_notifier/domains/killmail/supervisor.ex` around lines 81 - 94,
The GenServer currently calls WebSocketClient.start_link/0 directly (via
start_websocket_client()) which links the socket to this process; instead, start
the socket under the supervisor __MODULE__.InternalSupervisor (or via
Supervisor.start_child/2/DynamicSupervisor) so the socket is isolated from this
GenServer — replace direct calls to WebSocketClient.start_link/0 in
handle_continue(:start_websocket) and the retry path (the places calling
start_websocket_client and schedule_websocket_retry(state, 0)) with a function
that starts the child through __MODULE__.InternalSupervisor (e.g., build the
child_spec for WebSocketClient and call Supervisor.start_child or
DynamicSupervisor.start_child on __MODULE__.InternalSupervisor) and keep the
existing retry scheduling and {:noreply, state} / {:noreply,
schedule_websocket_retry(...)} return handling.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Duplicate comments:
In `@lib/wanderer_notifier/domains/killmail/supervisor.ex`:
- Around line 81-94: The GenServer currently calls WebSocketClient.start_link/0
directly (via start_websocket_client()) which links the socket to this process;
instead, start the socket under the supervisor __MODULE__.InternalSupervisor (or
via Supervisor.start_child/2/DynamicSupervisor) so the socket is isolated from
this GenServer — replace direct calls to WebSocketClient.start_link/0 in
handle_continue(:start_websocket) and the retry path (the places calling
start_websocket_client and schedule_websocket_retry(state, 0)) with a function
that starts the child through __MODULE__.InternalSupervisor (e.g., build the
child_spec for WebSocketClient and call Supervisor.start_child or
DynamicSupervisor.start_child on __MODULE__.InternalSupervisor) and keep the
existing retry scheduling and {:noreply, state} / {:noreply,
schedule_websocket_retry(...)} return handling.

In `@lib/wanderer_notifier/map/sse_supervisor.ex`:
- Around line 392-415: The retry logic in retry_failed_maps currently uses the
stale maps list captured earlier; before calling initialize_and_start_for_map
you should re-fetch current map configs (e.g. look up each map.slug or fetch all
live MapConfig entries) and only attempt initialization for configs that still
exist, skipping/logging maps that were removed or changed; update the code in
retry_failed_maps to map slugs -> fetch fresh configs, call
initialize_and_start_for_map with the fresh config, and then pass the remaining
failures into schedule_failed_map_retries (keep function names
retry_failed_maps, initialize_and_start_for_map, and schedule_failed_map_retries
for easy location).
- Around line 204-213: initialize_sse_clients/0 must always return :ok; after
calling run_parallel_init(maps), signal_pipeline_worker(),
start_sse_clients_staggered(successful_maps) and
schedule_failed_map_retries(failed_maps, 1), ignore and do not return the raw
result of schedule_failed_map_retries/2—explicitly return :ok. In
start_sse_clients_staggered/1 (and any code paths that call
Task.Supervisor.start_child/2 around lines referenced), catch failures from
Task.Supervisor.start_child/2 (use case/try/rescue or handle_info) so partial
startup errors are logged/handled locally and do not bubble up as {:error,
reason} from initialize_sse_clients/0; ensure any Task.Supervisor.start_child
results are normalized/logged and the outer initialize_sse_clients/0 still
returns :ok.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: a22b5f6c-713b-4464-be1d-1802771f38ca

📥 Commits

Reviewing files that changed from the base of the PR and between e3d7efb and 6a9a874.

📒 Files selected for processing (2)
  • lib/wanderer_notifier/domains/killmail/supervisor.ex
  • lib/wanderer_notifier/map/sse_supervisor.ex

guarzo and others added 2 commits April 19, 2026 20:24
- Start WebSocket as child of InternalSupervisor instead of linking
  directly to the GenServer — prevents WebSocket crash from taking
  down the Killmail Supervisor
- Ensure initialize_maps/0 and schedule_failed_map_retries/2 always
  return :ok; log Task.Supervisor.start_child failures explicitly
- Filter retry batch against current MapRegistry to skip maps
  removed since last attempt
- Extract filter_live_maps/1 and attempt_map_initialization/1 to
  satisfy credo ABC complexity limit

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Instead of filtering stale MapConfig structs by slug existence,
re-fetch current configs from MapRegistry on each retry attempt.
This ensures retries use up-to-date settings if a map's config
changed between attempts.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/wanderer_notifier/map/sse_supervisor.ex`:
- Around line 204-215: The current flow only adds maps that failed during
run_parallel_init/1 (i.e., initialize_map_data_safely/1 failures) to
schedule_failed_map_retries/2, but maps that fail later in
start_sse_client_for_map/1 (called by start_sse_clients_staggered/1) are merely
logged and never retried; modify start_sse_clients_staggered/1 so it returns the
list of %MapConfig{} entries whose SSE startup failed (preserving map context in
the Task.async_stream results), then merge that returned list with the
failed_maps from run_parallel_init/1 before calling
schedule_failed_map_retries/2 (i.e., call
start_sse_clients_staggered(successful_maps) -> {ok_started, sse_failed_maps}
and pass failed_maps ++ sse_failed_maps to schedule_failed_map_retries/2) so all
startup failures are retried.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 92236350-59fe-47e3-a3e4-f2c5fd765026

📥 Commits

Reviewing files that changed from the base of the PR and between 6a9a874 and c352bfb.

📒 Files selected for processing (2)
  • lib/wanderer_notifier/domains/killmail/supervisor.ex
  • lib/wanderer_notifier/map/sse_supervisor.ex

Comment thread lib/wanderer_notifier/map/sse_supervisor.ex

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

♻️ Duplicate comments (1)
lib/wanderer_notifier/map/sse_supervisor.ex (1)

210-213: ⚠️ Potential issue | 🟠 Major

SSE startup failures are still excluded from the retry queue.

initialize_maps/0 only schedules retries for failed_maps from data initialization. Failures from start_sse_clients_staggered/1 are logged but dropped, so those maps can still stay down until an external restart.

Proposed fix
 defp initialize_maps do
   maps = Dependencies.map_registry().all_maps()
   Logger.info("Initializing #{length(maps)} maps from registry", category: :startup)

   {successful_maps, failed_maps} = run_parallel_init(maps)

   # Signal PipelineWorker
   signal_pipeline_worker()

   # Start SSE clients only for successfully initialized maps
-  start_sse_clients_staggered(successful_maps)
+  sse_failed_maps = start_sse_clients_staggered(successful_maps)

   # Schedule retry for failed maps
-  schedule_failed_map_retries(failed_maps, 1)
+  schedule_failed_map_retries(failed_maps ++ sse_failed_maps, 1)

   :ok
 end

 defp start_sse_clients_staggered(maps) do
   results =
     maps
-    |> Task.async_stream(&start_sse_client_for_map/1,
+    |> Task.async_stream(fn map -> {map, start_sse_client_for_map(map)} end,
       max_concurrency: 5,
       timeout: 30_000
     )
     |> Enum.to_list()

   {succeeded, failed} =
     Enum.split_with(results, fn
-      {:ok, {:ok, _pid}} -> true
+      {:ok, {_map, {:ok, _pid}}} -> true
       _ -> false
     end)

   succeeded_count = length(succeeded)
   failed_count = length(failed)

+  failed_maps =
+    Enum.flat_map(failed, fn
+      {:ok, {%MapConfig{} = map, {:error, reason}}} ->
+        Logger.warning("SSE client startup failed",
+          map_slug: map.slug,
+          reason: inspect(reason)
+        )
+        [map]
+
+      {:exit, reason} ->
+        Logger.warning("SSE client startup task exited", reason: inspect(reason))
+        []
+
+      _ ->
+        []
+    end)
+
   if failed_count > 0 do
-    Enum.each(failed, fn
-      {:ok, {:error, reason}} ->
-        Logger.warning("SSE client startup failed", reason: inspect(reason))
-
-      {:exit, reason} ->
-        Logger.warning("SSE client startup task exited", reason: inspect(reason))
-
-      _ ->
-        :ok
-    end)
-
     Logger.warning(
       "SSE client startup: #{succeeded_count} succeeded, #{failed_count} failed out of #{length(maps)}",
       category: :startup
     )
   else
     Logger.info("Started #{succeeded_count} SSE clients", category: :startup)
   end
+
+  failed_maps
 end

Also applies to: 303-340

🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.

In `@lib/wanderer_notifier/map/sse_supervisor.ex` around lines 210 - 213,
initialize_maps/0 currently only calls schedule_failed_map_retries/2 with the
failed_maps returned from data initialization and ignores any maps that fail
during start_sse_clients_staggered/1; modify initialize_maps/0 (and the related
logic in the start_sse_clients_staggered flow) to collect and return or forward
any start-time failures (e.g., a failed_sse_maps list) and merge them with the
existing failed_maps before calling schedule_failed_map_retries/2 so that maps
which fail while starting SSE clients are enqueued for retry; apply the same
change for the analogous code region around the 303-340 block so all SSE startup
failures are scheduled for retry.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.

Inline comments:
In `@lib/wanderer_notifier/map/sse_supervisor.ex`:
- Around line 417-432: Refactor retry_failed_maps/2 into two function heads
using pattern matching: add a clause retry_failed_maps([], attempt) that logs
the "All previously failed maps..." message with attempt and category and
returns :ok, and a clause retry_failed_maps(maps, attempt) that logs "Retrying
initialization..." with length(maps), calls attempt_map_initialization(maps) to
get failed_maps and then calls schedule_failed_map_retries(failed_maps, attempt
+ 1); preserve the existing Logger.info messages, argument order and behavior of
attempt_map_initialization/1 and schedule_failed_map_retries/2.

---

Duplicate comments:
In `@lib/wanderer_notifier/map/sse_supervisor.ex`:
- Around line 210-213: initialize_maps/0 currently only calls
schedule_failed_map_retries/2 with the failed_maps returned from data
initialization and ignores any maps that fail during
start_sse_clients_staggered/1; modify initialize_maps/0 (and the related logic
in the start_sse_clients_staggered flow) to collect and return or forward any
start-time failures (e.g., a failed_sse_maps list) and merge them with the
existing failed_maps before calling schedule_failed_map_retries/2 so that maps
which fail while starting SSE clients are enqueued for retry; apply the same
change for the analogous code region around the 303-340 block so all SSE startup
failures are scheduled for retry.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: ASSERTIVE

Plan: Pro

Run ID: 96d3cb6e-66c0-4a14-a07e-5ea74c9a7e94

📥 Commits

Reviewing files that changed from the base of the PR and between c352bfb and c1fe25f.

📒 Files selected for processing (1)
  • lib/wanderer_notifier/map/sse_supervisor.ex

Comment thread lib/wanderer_notifier/map/sse_supervisor.ex Outdated
- start_sse_clients_staggered now returns maps that failed SSE client
  startup; these are merged with data-init failures before scheduling
  retries, so no failure path is silently dropped
- retry_failed_maps split into two function heads ([], attempt) and
  (maps, attempt) with fetch_fresh_configs moved to the caller
- Extract log_sse_startup_results/3 to satisfy credo ABC limit

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@guarzo
guarzo merged commit 2f47de0 into main Apr 19, 2026
5 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant