Fix admin panel hanging when the admin user storage is unreachable - #904
Merged
Conversation
The authorization of every request asks whether an admin user exists, so that the panel stays reachable until the first user was created. That question hit the database, and both steps of it were unbounded: - The repository ran the schema migration on every attempt, without a timeout and without backing off after a failure. The connection string allows a command timeout of two minutes. - While the first caller was stuck in that attempt, every other caller queued up behind the semaphore of the availability check - including the requests which render the page. So on a system whose database was not reachable yet - right after an update, or while the installation is still running - the panel never finished loading. The navigation stayed at "Loading ...", which also hides the setup entry, so the installation couldn't even be started. Now: - The availability check never waits for a probe which is already running; it answers with what is known instead. - The connection is checked separately with a short timeout before a migration is attempted, the migration has a timeout of its own, and after a failure the storage is only probed again after a delay. - When the storage is unreachable, the previous answer is kept instead of being treated as "no user exists". The regression test fails against the previous implementation. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_015MLJ586D4tVVznbF5g7MHp
The containers run as a non-root user while /app belongs to root, and only /app/logs was made writable. The data protection key ring was configured to be stored in /app/data-protection-keys, which that user can't create - so as soon as a key was needed, which is the case for every antiforgery token and for the authentication cookie, the request failed and the panel showed its error page. Mounting the volume of the compose files doesn't help either, because docker seeds a new named volume from the image and it inherits the same ownership. - Both images which host the admin panel create the directory and make it writable before they drop to the non-root user. - The key directory is created and probed for write access when the application starts. If it can't be used, the panel now logs a warning naming the path and falls back to keys which are only kept in memory, instead of failing every request. That's a degradation - everybody is signed out on a restart and stored authenticator keys become unreadable - but it keeps the panel usable, which is what somebody in that situation needs in order to fix it. The test protects a value through the configured provider with an unusable key directory; it fails when the keys are persisted to that path unconditionally. Co-Authored-By: Claude Opus 5 <[email protected]> Claude-Session: https://claude.ai/code/session_015MLJ586D4tVVznbF5g7MHp
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Follow-up to #898, from user feedback: after updating, the admin panel never finished loading. The navigation stayed at Loading …, and since the setup entry lives in the branch that is hidden while loading, the installation could not even be started.
What went wrong
The authorization of every request asks whether an admin user exists, so that the panel stays reachable until the first user has been created. Both halves of that question were unbounded:
AdminUserRepository.EnsureStorageAsyncranDatabase.MigrateAsync()on every attempt, with no timeout and no backoff after a failure. The connection string allowsCommand Timeout=120.AdminUserAvailabilityService— including the requests which render the page. The negative result was only cached after the probe returned, so nothing short-circuited while it was running.On a system whose database was not reachable yet — right after an update, or while the installation is still running — the circuit's renders stalled behind a connection attempt that was going to time out anyway.
Fix
WaitAsync(0)); it answers with what is known instead.Testing
AvailabilityCheckDoesNotWaitForARunningProbeAsyncis the actual regression test: a repository blocks its availability check on aTaskCompletionSource, and the test asserts that a concurrent check completes synchronously instead of queuing behind it. Verified that it fails against the previous implementation and passes against this one.Two supporting tests cover that an unreachable storage is not probed on every check, and that an existing user is remembered.
My first attempt at a regression test would have passed against the broken code as well — the existing five second cache meant rapid repeated calls only probed once either way — so it is not in this change.
MUnique.OpenMU.Web.Tests: 29/29 pass (3 new).MUnique.OpenMU.Tests: 769/769 pass.Not verified
Still no database in the environment this was developed in, so the behaviour against a real unreachable PostgreSQL has not been exercised end to end — the tests drive the blocking through a repository double. If the panel still hangs after this, the server log is the next place to look:
EnsureStorageAsynclogs an Information entry with the connection exception each time it gives up.🤖 Generated with Claude Code
https://claude.ai/code/session_015MLJ586D4tVVznbF5g7MHp
Generated by Claude Code