Skip to content

Fix critical supervisor ordering issue in application.ex - #193

Merged
ddon merged 5 commits into
BeamLabEU:devfrom
timujinne:dev
Nov 23, 2025
Merged

Fix critical supervisor ordering issue in application.ex#193
ddon merged 5 commits into
BeamLabEU:devfrom
timujinne:dev

Conversation

@timujinne

Copy link
Copy Markdown
Contributor

Summary

Addresses critical startup crash caused by incorrect supervisor ordering during PhoenixKit installation.

Problem

PhoenixKit.Supervisor and Oban were starting before Ecto Repo, causing:

  • Settings cache failures (Repo not ready)
  • Oban crashes (database not available)
  • Application startup failures

Changes

1. Installation Logic (application_supervisor.ex)

  • Added explicit after: [repo] positioning using Igniter API
  • Guarantees correct order: Repo → PhoenixKit → Oban → Endpoint
  • Added comprehensive documentation with examples

2. Oban Configuration (oban_config.ex)

  • Replaced text-based injection with proper Igniter Application.add_new_child
  • Uses explicit positioning: after: [PhoenixKit.Supervisor]
  • Simplified code (removed ~70 lines of text manipulation)

3. Automatic Fix (phoenix_kit.update)

  • Added fix_supervisor_ordering/1 function
  • Detects incorrect ordering in existing installations
  • Automatic reordering with user notices
  • Handles edge cases (missing Repo, unusual setups)

4. Documentation

  • Added comprehensive troubleshooting guide (guides/troubleshooting.md)
  • Updated CHANGELOG with critical fix notice
  • Documented problem, solution, and prevention

Impact

New Installations:

  • ✅ Correct supervisor order guaranteed
  • ✅ No startup crashes
  • ✅ Settings cache works immediately

Existing Installations:

  • ✅ Automatic detection in update command
  • ✅ Manual fix instructions in troubleshooting guide
  • ✅ Clear error messages and solutions

Testing

  • ✅ Code quality checks passed (Credo, Dialyzer, format)
  • ✅ Compilation successful with warnings-as-errors
  • ✅ Documented in troubleshooting guide

Migration Path

Users experiencing startup crashes can:

  1. Run mix phoenix_kit.update (attempts automatic fix)
  2. Follow manual fix in guides/troubleshooting.md
  3. Correct order: Repo → PhoenixKit.Supervisor → Oban

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
…ix 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
@ddon
ddon merged commit bd6cd86 into BeamLabEU:dev Nov 23, 2025
6 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.

2 participants