From b3c071b0e5aed2c02d0f7a4252aa08bc9ad8768a Mon Sep 17 00:00:00 2001 From: timujeen Date: Sat, 22 Nov 2025 22:03:27 +0000 Subject: [PATCH 1/2] Fix Oban config detection and add automatic restart This commit fixes the infinite loop issue where mix phoenix_kit.update would get stuck checking for Oban configuration and never execute migrations. Problems fixed: 1. has_active_oban_config? was checking for hardcoded app name - BEFORE: Searched for "config :phoenix_kit, Oban" - AFTER: Searches for ", Oban" to match any app name - This fix applied to both install.ex and update.ex 2. No automatic restart after adding configuration - Commands now automatically restart instead of asking user - Uses is_retry flag and recursive run(argv) call - Prevents infinite loops with {:missing, true} safety check 3. Removed unused code that caused compilation warnings - Removed unused 'result' variable in both files - Removed show_config_added_message/1 function (no longer needed) Technical implementation: - Pattern match on {config_status, is_retry} tuple - First pass {:missing, false}: Add config + automatic restart - Second pass {:ok, _}: Execute migrations normally - Safety check {:missing, true}: Prevent infinite loops Results: - mix phoenix_kit.update now completes in one command - Automatically detects missing Oban config (any app name) - Automatically adds config and reruns - Executes migrations without manual intervention - No more "run again" messages - fully automatic Also includes: - Comment explaining igniter usage in mix.exs --- .dialyzer_ignore.exs | 2 + CHANGELOG.md | 11 + lib/mix/tasks/phoenix_kit.install.ex | 69 +++- lib/mix/tasks/phoenix_kit.update.ex | 335 ++++++++++++++++-- .../install/application_supervisor.ex | 43 ++- lib/phoenix_kit/install/oban_config.ex | 75 ++-- mix.exs | 2 + 7 files changed, 442 insertions(+), 95 deletions(-) diff --git a/.dialyzer_ignore.exs b/.dialyzer_ignore.exs index 80ef9f010..f60ff67b3 100644 --- a/.dialyzer_ignore.exs +++ b/.dialyzer_ignore.exs @@ -2,6 +2,8 @@ # Mix functions are only available during Mix compilation context {"lib/mix/tasks/phoenix_kit.install.ex", :unknown_function}, {"lib/mix/tasks/phoenix_kit.update.ex", :unknown_function}, + # Conditional compilation pattern match in update.ex (Code.ensure_loaded?) + {"lib/mix/tasks/phoenix_kit.update.ex", :pattern_match, 1}, {"lib/mix/tasks/phoenix_kit.modernize_layouts.ex", :unknown_function}, {"lib/phoenix_kit/install/migration_strategy.ex", :unknown_function}, {"lib/phoenix_kit/install/repo_detection.ex", :unknown_function}, diff --git a/CHANGELOG.md b/CHANGELOG.md index 37e198b0a..b69efeab5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,14 @@ +## [Unreleased] + +### Fixed +- **CRITICAL: Supervisor Ordering** - Fixed startup crashes caused by incorrect supervisor order in application.ex + - PhoenixKit.Supervisor and Oban now correctly start AFTER Repo instead of before + - Added explicit positioning using `after: [repo]` in Igniter installation logic + - Replaced text-based supervisor injection with proper Igniter.Project.Application API + - Added automatic fix in `mix phoenix_kit.update` to correct existing installations + - Prevents crashes: "Repo not ready" errors when loading Settings cache or Oban jobs + - Ensures correct order: Repo → PhoenixKit.Supervisor → Oban → Endpoint + ## 1.6.7 - 2025-11-22 - Fixed redundant copies, label and database file_location row generation issue when uploading images - Refactor Application usage and Modules names diff --git a/lib/mix/tasks/phoenix_kit.install.ex b/lib/mix/tasks/phoenix_kit.install.ex index 23c43a078..4ad875c35 100644 --- a/lib/mix/tasks/phoenix_kit.install.ex +++ b/lib/mix/tasks/phoenix_kit.install.ex @@ -135,18 +135,40 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do # CRITICAL: Check if required configuration exists BEFORE starting app # This prevents configuration timing issues where config is added via Igniter # but the app has already started with cached (missing) configuration + + # Check if this is a retry pass (automatic restart after adding config) + is_retry = Process.get(:phoenix_kit_retry_pass, false) config_status = check_required_configuration() - case config_status do - :missing -> + case {config_status, is_retry} do + {:missing, false} -> # First pass: Add configuration via Igniter without starting app + # Store status in Process dictionary for tracking + Process.put(:phoenix_kit_config_status, :missing) + show_missing_config_message(argv) - result = super(argv) - show_config_added_message(argv) - result + super(argv) + + # AUTOMATIC RESTART instead of asking user to run again manually + Mix.shell().info(""" + + ✅ Configuration added successfully! + 🔄 Automatically restarting to complete the installation... + """) - :ok -> + # Clean Process dictionary to ensure fresh state for retry + Process.delete(:phoenix_kit_config_status) + + # Mark this as a retry pass to prevent infinite loops + Process.put(:phoenix_kit_retry_pass, true) + + # Recursive call with same arguments - automatic restart + run(argv) + + {:ok, _} -> # Second pass: Configuration exists, safe to start app and complete installation + Process.put(:phoenix_kit_config_status, :ok) + # Run standard igniter process result = super(argv) @@ -158,7 +180,27 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do AssetRebuild.check_and_rebuild(verbose: true) end + # Clean up retry flag on successful completion + Process.delete(:phoenix_kit_retry_pass) result + + {:missing, true} -> + # Safety check: Configuration still missing after automatic retry + # This prevents infinite loops if configuration addition fails + Mix.shell().error(""" + + ❌ Configuration was not added successfully after automatic retry. + + Please check config/config.exs manually and ensure it contains: + - config :ueberauth, Ueberauth (with providers: %{}) + - config :hammer (with backend and expiry_ms) + - config :phoenix_kit, Oban (with queues configuration) + + Then run: mix phoenix_kit.install #{Enum.join(argv, " ")} + """) + + Process.delete(:phoenix_kit_retry_pass) + :error end end end @@ -286,17 +328,6 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do """) end - # Display message after configuration is added - defp show_config_added_message(argv) do - Mix.shell().info(""" - - ✅ Configuration added successfully! - - Next step: Run the install command again to complete the installation: - mix phoenix_kit.install #{Enum.join(argv, " ")} - """) - end - # Check if all required configuration exists # Returns :ok if all config present, :missing if any config is missing defp check_required_configuration do @@ -361,9 +392,9 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do has_oban_config = Enum.any?(lines, fn line -> trimmed = String.trim(line) - # Not a comment and contains config :phoenix_kit, Oban + # Not a comment and contains config :any_app, Oban !String.starts_with?(trimmed, "#") and - String.contains?(line, "config :phoenix_kit, Oban") + String.contains?(line, ", Oban") end) has_queues = diff --git a/lib/mix/tasks/phoenix_kit.update.ex b/lib/mix/tasks/phoenix_kit.update.ex index 1429dd62d..8903fd1a6 100644 --- a/lib/mix/tasks/phoenix_kit.update.ex +++ b/lib/mix/tasks/phoenix_kit.update.ex @@ -88,6 +88,7 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do BasicConfiguration, Common, CssIntegration, + IgniterHelpers, ObanConfig, RateLimiterConfig } @@ -167,26 +168,60 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do # CRITICAL: Check if required configuration exists BEFORE starting app # This prevents configuration timing issues where config is added via Igniter # but the app has already started with cached (missing) configuration + + # Check if this is a retry pass (automatic restart after adding config) + is_retry = Process.get(:phoenix_kit_retry_pass, false) config_status = check_required_configuration() - case config_status do - :missing -> + case {config_status, is_retry} do + {:missing, false} -> # First pass: Add configuration via Igniter without starting app # Store config status in Process dictionary for igniter/1 to read Process.put(:phoenix_kit_config_status, :missing) show_missing_config_message(argv) - result = super(argv) - show_config_added_message(argv) - result + super(argv) + + # Automatic restart instead of manual prompt + Mix.shell().info(""" + + ✅ Configuration added successfully! + 🔄 Automatically restarting to complete the update... + """) + + # Clean Process dictionary for fresh state + Process.delete(:phoenix_kit_config_status) + Process.put(:phoenix_kit_retry_pass, true) + + # Recursive call with same arguments + run(argv) - :ok -> - # Second pass: Configuration exists, safe to start app and update + {:ok, _} -> + # Second pass (automatic or manual): Configuration exists, safe to start app # Store config status in Process dictionary for igniter/1 to read Process.put(:phoenix_kit_config_status, :ok) Mix.Task.run("app.start") result = super(argv) post_igniter_tasks(elem(opts, 0)) + + # Clean retry flag + Process.delete(:phoenix_kit_retry_pass) result + + {:missing, true} -> + # Safety: Configuration still missing after retry + Mix.shell().error(""" + + ❌ Configuration was not added successfully after automatic retry. + + This may indicate a problem with your config/config.exs file. + Please check the file manually and ensure it's writable. + + Then run manually: + mix phoenix_kit.update #{Enum.join(argv, " ")} + """) + + Process.delete(:phoenix_kit_retry_pass) + :error end end end @@ -210,17 +245,6 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do """) end - # Display message after configuration is added - defp show_config_added_message(argv) do - Mix.shell().info(""" - - ✅ Configuration added successfully! - - Next step: Run the update command again to complete the upgrade: - mix phoenix_kit.update #{Enum.join(argv, " ")} - """) - end - # Check if all required configuration exists # Returns :ok if all config present, :missing if any config is missing defp check_required_configuration do @@ -285,9 +309,10 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do has_oban_config = Enum.any?(lines, fn line -> trimmed = String.trim(line) - # Not a comment and contains config :phoenix_kit, Oban + # Not a comment and contains config for any app with Oban + # Matches: "config :any_app, Oban" or "config :any_app, Oban," !String.starts_with?(trimmed, "#") and - String.contains?(line, "config :phoenix_kit, Oban") + String.contains?(line, ", Oban") end) has_queues = @@ -314,6 +339,10 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do # Ensure Oban configuration exists igniter = validate_and_add_oban_config(igniter) + # CRITICAL FIX: Ensure correct supervisor ordering in application.ex + # This must run AFTER add_oban_supervisor to fix installations with wrong order + igniter = fix_supervisor_ordering(igniter) + # Check if this is the first pass (config missing) or second pass (config exists) config_status = Process.get(:phoenix_kit_config_status, :ok) @@ -891,6 +920,272 @@ if Code.ensure_loaded?(Igniter.Mix.Task) do end # Validate and add Oban configuration if missing + # Fix supervisor ordering in application.ex to prevent startup crashes + # Ensures correct order: Repo → PhoenixKit.Supervisor → Oban → Endpoint + defp fix_supervisor_ordering(igniter) do + app_name = IgniterHelpers.get_parent_app_name(igniter) + app_file = "lib/#{app_name}/application.ex" + + if File.exists?(app_file) do + content = File.read!(app_file) + + # Check current supervisor ordering + case check_supervisor_order(content, app_name) do + :correct -> + # Order is already correct, no changes needed + igniter + + {:needs_fix, reason} -> + # Order is incorrect, attempt to fix using Igniter API + igniter + |> fix_application_supervisor_order(app_name, reason) + |> add_supervisor_ordering_fixed_notice(reason) + + :cannot_determine -> + # Cannot determine order (unusual setup), skip silently + igniter + end + else + # No application.ex found (unusual), skip + igniter + end + rescue + e -> + # If any error occurs, log warning but continue + Mix.shell().info("⚠️ Could not check supervisor ordering: #{inspect(e)}") + igniter + end + + # Check the ordering of supervisors in application.ex + # Returns :correct, {:needs_fix, reason}, or :cannot_determine + defp check_supervisor_order(content, app_name) do + lines = String.split(content, "\n") + + # Find line numbers for each supervisor + repo_line = find_supervisor_line(lines, ~r/#{app_name}\.Repo[,\s]/) + phoenix_kit_line = find_supervisor_line(lines, ~r/PhoenixKit\.Supervisor[,\s]/) + + oban_line = + find_supervisor_line(lines, ~r/\{Oban,|Application\.get_env\(:#{app_name}, Oban\)/) + + validate_supervisor_positions(repo_line, phoenix_kit_line, oban_line) + end + + # Validate supervisor positions and return check result + defp validate_supervisor_positions(nil, nil, nil), do: :cannot_determine + defp validate_supervisor_positions(nil, _, _), do: :cannot_determine + defp validate_supervisor_positions(repo, nil, nil) when is_integer(repo), do: :correct + + defp validate_supervisor_positions(repo, pk, nil) when is_integer(repo) and is_integer(pk) do + if repo < pk, do: :correct, else: {:needs_fix, "PhoenixKit.Supervisor before Repo"} + end + + defp validate_supervisor_positions(repo, pk, oban) + when is_integer(repo) and is_integer(pk) and is_integer(oban) do + check_three_supervisor_order(repo, pk, oban) + end + + defp validate_supervisor_positions(_, _, _), do: :cannot_determine + + # Check ordering when all three supervisors exist + defp check_three_supervisor_order(repo, pk, oban) do + cond do + pk < repo and oban < repo -> {:needs_fix, "both PhoenixKit and Oban before Repo"} + pk < repo -> {:needs_fix, "PhoenixKit.Supervisor before Repo"} + oban < repo -> {:needs_fix, "Oban before Repo"} + oban < pk -> {:needs_fix, "Oban before PhoenixKit.Supervisor"} + true -> :correct + end + end + + # Find the line number where a supervisor is defined + defp find_supervisor_line(lines, pattern) do + lines + |> Enum.with_index(1) + |> Enum.find(fn {line, _index} -> + trimmed = String.trim(line) + # Not a comment and matches pattern + !String.starts_with?(trimmed, "#") and Regex.match?(pattern, line) + end) + |> case do + {_line, index} -> index + nil -> nil + end + end + + # Fix the supervisor ordering using manual reordering + # Note: We can't use Igniter.Project.Application.add_new_child to reorder existing children, + # so we need to manually reorder the children list + defp fix_application_supervisor_order(igniter, app_name, _reason) do + app_file = "lib/#{app_name}/application.ex" + + Igniter.update_file(igniter, app_file, fn source -> + content = Rewrite.Source.get(source, :content) + fixed_content = reorder_supervisors(content, app_name) + Rewrite.Source.update(source, :content, fixed_content) + end) + end + + # Reorder supervisors in application.ex to correct order + defp reorder_supervisors(content, app_name) do + lines = String.split(content, "\n") + + # Extract supervisor lines + {repo_line, repo_index} = extract_supervisor(lines, ~r/#{app_name}\.Repo[,\s]/) + {pk_line, pk_index} = extract_supervisor(lines, ~r/PhoenixKit\.Supervisor[,\s]/) + + {oban_line, oban_index} = + extract_supervisor(lines, ~r/\{Oban,|Application\.get_env\(:#{app_name}, Oban\)/) + + # Determine children list boundaries + children_start = find_children_list_start(lines) + children_end = find_children_list_end(lines, children_start) + + if is_integer(children_start) and is_integer(children_end) do + # Build new children list with correct order + supervisors = %{ + repo: {repo_line, repo_index}, + phoenix_kit: {pk_line, pk_index}, + oban: {oban_line, oban_index} + } + + new_lines = + rebuild_children_list(lines, children_start, children_end, supervisors) + + Enum.join(new_lines, "\n") + else + # Cannot find children list boundaries, return unchanged + content + end + end + + # Extract supervisor line and its index + defp extract_supervisor(lines, pattern) do + case Enum.with_index(lines, 1) do + indexed_lines -> + case Enum.find(indexed_lines, fn {line, _index} -> + trimmed = String.trim(line) + !String.starts_with?(trimmed, "#") and Regex.match?(pattern, line) + end) do + {line, index} -> {line, index} + nil -> {nil, nil} + end + end + end + + # Find the start of children list + defp find_children_list_start(lines) do + Enum.find_index(lines, fn line -> + String.contains?(line, "children = [") + end) + end + + # Find the end of children list (closing bracket) + defp find_children_list_end(lines, start_index) do + lines + |> Enum.drop(start_index + 1) + |> Enum.with_index(start_index + 1) + |> Enum.find(fn {line, _index} -> + trimmed = String.trim(line) + trimmed == "]" + end) + |> case do + {_line, index} -> index + nil -> nil + end + end + + # Rebuild children list with correct supervisor ordering + defp rebuild_children_list(lines, children_start, children_end, supervisors) do + %{ + repo: {repo_line, repo_index}, + phoenix_kit: {pk_line, pk_index}, + oban: {oban_line, oban_index} + } = supervisors + + # Lines before children list + before_children = Enum.take(lines, children_start + 1) + + # Lines after children list + after_children = Enum.drop(lines, children_end) + + # Get all children between start and end + children_lines = + lines + |> Enum.drop(children_start + 1) + |> Enum.take(children_end - children_start - 1) + + # Remove repo, phoenix_kit, and oban lines from children + filtered_children = + children_lines + |> Enum.with_index(children_start + 2) + |> Enum.reject(fn {_line, index} -> + index in [repo_index, pk_index, oban_index] + end) + |> Enum.map(fn {line, _index} -> line end) + + # Build new ordered children list + ordered_children = + build_ordered_supervisor_list(repo_line, pk_line, oban_line, filtered_children) + + # Reconstruct file + before_children ++ ordered_children ++ after_children + end + + # Build ordered list of supervisors with correct positioning + defp build_ordered_supervisor_list(repo_line, pk_line, oban_line, filtered_children) do + # Add Repo first (if exists) + ordered = if repo_line, do: [repo_line], else: [] + + # Split remaining children at Endpoint + {before_endpoint, from_endpoint} = split_at_endpoint(filtered_children) + + # Add PhoenixKit after Repo, before Endpoint + ordered = ordered ++ before_endpoint + ordered = if pk_line, do: ordered ++ [pk_line], else: ordered + + # Add Oban after PhoenixKit + ordered = if oban_line, do: ordered ++ [oban_line], else: ordered + + # Add remaining children (Endpoint and after) + ordered ++ from_endpoint + end + + # Split children at Endpoint line + defp split_at_endpoint(children) do + endpoint_index = + Enum.find_index(children, fn line -> + String.contains?(line, "Endpoint") and !String.contains?(line, "#") + end) + + case endpoint_index do + nil -> {children, []} + index -> Enum.split(children, index) + end + end + + # Add notice about supervisor ordering being fixed + defp add_supervisor_ordering_fixed_notice(igniter, reason) do + notice = """ + ⚠️ CRITICAL FIX APPLIED: Corrected supervisor ordering in application.ex + + Issue detected: #{reason} + + Fixed to correct order: + 1. YourApp.Repo (database connection - must be first) + 2. PhoenixKit.Supervisor (uses Repo for Settings cache) + 3. {Oban, ...} (uses Repo for job persistence) + 4. Other supervisors... + + This fixes startup crashes where PhoenixKit or Oban tried to access + the database before Repo was ready. + + IMPORTANT: Restart your server for changes to take effect. + """ + + Igniter.add_notice(igniter, String.trim(notice)) + end + defp validate_and_add_oban_config(igniter) do config_exists = ObanConfig.oban_config_exists?(igniter) supervisor_exists = ObanConfig.oban_supervisor_exists?(igniter) diff --git a/lib/phoenix_kit/install/application_supervisor.ex b/lib/phoenix_kit/install/application_supervisor.ex index 1d612c334..53279dd9d 100644 --- a/lib/phoenix_kit/install/application_supervisor.ex +++ b/lib/phoenix_kit/install/application_supervisor.ex @@ -2,18 +2,55 @@ defmodule PhoenixKit.Install.ApplicationSupervisor do @moduledoc """ Installation helper for adding PhoenixKit supervisor to parent application. Used by `mix phoenix_kit.install` task. + + ## Important + + PhoenixKit.Supervisor MUST start AFTER the Ecto Repo because it depends on + the database for loading Settings cache and OAuth configuration. + + Incorrect order (will crash): + ```elixir + children = [ + PhoenixKit.Supervisor, # ❌ Tries to read Settings from DB + MyApp.Repo # ⚠️ DB not ready yet! + ] + ``` + + Correct order: + ```elixir + children = [ + MyApp.Repo, # ✅ Start DB first + PhoenixKit.Supervisor # ✅ Then PhoenixKit + ] + ``` """ use PhoenixKit.Install.IgniterCompat + alias Igniter.Libs.Ecto alias Igniter.Libs.Phoenix alias Igniter.Project.Application def add_supervisor(igniter) do {igniter, endpoint} = Phoenix.select_endpoint(igniter) + {igniter, repos} = Ecto.list_repos(igniter) + + repo = List.first(repos) + + # Build positioning options based on whether we found a Repo + opts = + case repo do + nil -> + # No Repo found - just add before endpoint + # User will need to manually reorder if they have a Repo + [before: [endpoint]] + + repo_module -> + # Repo found - explicitly position AFTER Repo AND BEFORE Endpoint + # This ensures correct startup order: Repo → PhoenixKit → Endpoint + [after: [repo_module], before: [endpoint]] + end igniter - |> Application.add_new_child(PhoenixKit.Supervisor, - before: [endpoint] - ) + |> Application.add_new_child(PhoenixKit.Supervisor, opts) end end diff --git a/lib/phoenix_kit/install/oban_config.ex b/lib/phoenix_kit/install/oban_config.ex index 901479c52..08de9dbe1 100644 --- a/lib/phoenix_kit/install/oban_config.ex +++ b/lib/phoenix_kit/install/oban_config.ex @@ -11,6 +11,8 @@ defmodule PhoenixKit.Install.ObanConfig do """ use PhoenixKit.Install.IgniterCompat + alias Igniter.Libs.Phoenix + alias Igniter.Project.Application alias PhoenixKit.Install.IgniterHelpers @doc """ @@ -258,8 +260,17 @@ defmodule PhoenixKit.Install.ObanConfig do Adds Oban to the parent application's supervision tree. This function ensures that Oban starts automatically when the application starts, - positioned after PhoenixKit.Supervisor to ensure PhoenixKit services are available - before Oban workers run. + with correct positioning in the supervisor tree: + - AFTER PhoenixKit.Supervisor (PhoenixKit services available) + - BEFORE Endpoint (Oban ready before HTTP requests) + + ## Important + + Oban MUST start AFTER PhoenixKit.Supervisor because PhoenixKit.Supervisor + depends on Repo, and Oban also depends on Repo. The correct order is: + 1. Repo (database connection) + 2. PhoenixKit.Supervisor (uses Repo for Settings) + 3. Oban (uses Repo for job persistence) ## Parameters - `igniter` - The igniter context @@ -269,58 +280,16 @@ defmodule PhoenixKit.Install.ObanConfig do """ def add_oban_supervisor(igniter) do app_name = IgniterHelpers.get_parent_app_name(igniter) - app_file = "lib/#{app_name}/application.ex" - oban_line = " {Oban, Application.get_env(:#{app_name}, Oban)}," - - Igniter.update_file(igniter, app_file, fn source -> - update_application_with_oban(source, app_name, oban_line) - end) - end - - # Update application.ex source with Oban supervisor - defp update_application_with_oban(source, app_name, oban_line) do - content = Rewrite.Source.get(source, :content) - - if oban_already_configured?(content, app_name) do - source - else - updated_content = insert_or_replace_oban(content, oban_line) - Rewrite.Source.update(source, :content, updated_content) - end - end - - # Check if Oban is already properly configured - defp oban_already_configured?(content, app_name) do - String.contains?(content, "{Oban, Application.get_env(:#{app_name}, Oban)}") - end + {igniter, endpoint} = Phoenix.select_endpoint(igniter) - # Insert or replace Oban configuration in application.ex - defp insert_or_replace_oban(content, oban_line) do - lines = String.split(content, "\n") - has_bare_oban = Enum.any?(lines, fn line -> String.trim(line) == "Oban," end) - - updated_lines = process_application_lines(lines, oban_line, has_bare_oban) - Enum.join(updated_lines, "\n") - end - - # Process each line to insert/replace Oban - defp process_application_lines(lines, oban_line, has_bare_oban) do - Enum.reduce(lines, [], fn line, acc -> - trimmed = String.trim(line) - - cond do - # Replace bare "Oban," with proper config - trimmed == "Oban," -> - acc ++ [oban_line] - - # Insert Oban after PhoenixKit.Supervisor ONLY if there's no bare Oban to replace - String.contains?(line, "PhoenixKit.Supervisor") and not has_bare_oban -> - acc ++ [line, oban_line] - - true -> - acc ++ [line] - end - end) + # Use Igniter API to add Oban with explicit positioning + # This ensures correct order: Repo → PhoenixKit → Oban → Endpoint + igniter + |> Application.add_new_child( + {Oban, {:application, app_name, Oban}}, + after: [PhoenixKit.Supervisor], + before: [endpoint] + ) end @doc """ diff --git a/mix.exs b/mix.exs index 66a80709f..03d8a52fa 100644 --- a/mix.exs +++ b/mix.exs @@ -128,6 +128,8 @@ defmodule PhoenixKit.MixProject do {:finch, "~> 0.18"}, # Code generation and project patching + # Note: Available in all environments for library code, but typically + # only needed in :dev when used as a dependency in parent projects {:igniter, "~> 0.7"} ] end From d25609f123c5ffd419bbe44967d54312ee63fabb Mon Sep 17 00:00:00 2001 From: timujeen Date: Sun, 23 Nov 2025 14:37:58 +0000 Subject: [PATCH 2/2] Fix Settings cache and OAuth config loader graceful handling during Mix tasks Changes: - Settings cache checks repo availability before warming - OAuth config loader reduced debug messages from 10 to 3 - Changed OAuth warning to debug level for Mix tasks - Removed verbose cache size logging during retries Impact: - Eliminated error: Failed to warm settings cache - Reduced noise from 20+ to 4 debug messages - Changed warning to debug for expected scenarios - Clean output for mix phoenix_kit.status --- lib/phoenix_kit/settings/settings.ex | 38 +++++++++++-------- .../workers/oauth_config_loader.ex | 20 ++++++---- 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/lib/phoenix_kit/settings/settings.ex b/lib/phoenix_kit/settings/settings.ex index 8f81fb614..25483d92d 100644 --- a/lib/phoenix_kit/settings/settings.ex +++ b/lib/phoenix_kit/settings/settings.ex @@ -1299,21 +1299,29 @@ defmodule PhoenixKit.Settings do Prioritizes JSON values over string values for cache storage. """ def warm_cache_data do - settings = repo().all(Setting) - - settings - |> Enum.map(fn setting -> - # Prioritize JSON value over string value for cache storage - value = - if setting.value_json do - setting.value_json - else - setting.value - end - - {setting.key, value} - end) - |> Map.new() + # Check if repository is available before attempting to warm cache + # This prevents errors during Mix tasks when repo might not be started yet + if repo_available?() do + settings = repo().all(Setting) + + settings + |> Enum.map(fn setting -> + # Prioritize JSON value over string value for cache storage + value = + if setting.value_json do + setting.value_json + else + setting.value + end + + {setting.key, value} + end) + |> Map.new() + else + # Repo not available (likely during Mix task execution) + # Return empty map - cache will be warmed later when repo becomes available + %{} + end rescue error -> Logger.error("Failed to warm settings cache: #{inspect(error)}") diff --git a/lib/phoenix_kit/workers/oauth_config_loader.ex b/lib/phoenix_kit/workers/oauth_config_loader.ex index 7889f3119..3efb1849a 100644 --- a/lib/phoenix_kit/workers/oauth_config_loader.ex +++ b/lib/phoenix_kit/workers/oauth_config_loader.ex @@ -110,8 +110,10 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do {:ok, %{status: :loaded}} {:error, :cache_not_ready} -> - Logger.warning( - "OAuth config loading failed after #{@max_retries} attempts: cache not ready" + # During Mix tasks (like phoenix_kit.update --status), the cache may not be ready + # This is expected and not an error condition - log as debug instead of warning + Logger.debug( + "OAuth config loading skipped: Settings cache not ready (likely during Mix task execution)" ) # Don't fail supervisor startup if cache is not ready @@ -162,7 +164,13 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do :ok {:error, :cache_not_ready} when attempt < @max_retries -> - Logger.debug("Settings cache not ready, retrying... (attempt #{attempt}/#{@max_retries})") + # Only log every 3rd attempt to reduce noise during Mix tasks + if rem(attempt, 3) == 0 do + Logger.debug( + "Settings cache not ready, retrying... (attempt #{attempt}/#{@max_retries})" + ) + end + Process.sleep(@retry_delay) load_oauth_config_with_retry(attempt + 1) @@ -192,10 +200,8 @@ defmodule PhoenixKit.Workers.OAuthConfigLoader do cache_size = get_cache_size() if cache_size < 40 do - Logger.debug( - "Settings cache not fully warmed yet (#{cache_size} entries, expected 40+), retrying..." - ) - + # Don't log every attempt to reduce noise during Mix tasks + # Cache will be empty during Mix tasks when repo is not available {:error, :cache_not_ready} else # Strategy 2: Verify OAuth-specific settings are present