Repository navigation
Commit bb0837a
authored
fix: shutdown-noise in background services + seed defaults so the app starts out of the box (#279)
* fix: stop background services reporting shutdown as a crash
A host that aborts startup cancels background services mid-wait. Three
services awaited a delay outside any handler for that cancellation, so the
TaskCanceledException escaped ExecuteAsync. .NET treats that as a crashed
BackgroundService and, under the default StopHost behaviour, logs a critical
"BackgroundService failed" on top of the error that actually stopped the host
— burying the real cause.
AuditRetentionService was the worst case: its one-minute startup delay had no
try at all, so any shutdown in the first 60 seconds of process life tripped it.
The interval delay at the foot of its loop sat outside the try as well; a
single outer handler now covers both.
AuditWriterService and ProgressFlushService each awaited a 5s error backoff
from inside a catch block, where the enclosing try cannot observe it. Those
are guarded locally and now break out of the loop on shutdown.
Behaviour is unchanged on every non-shutdown path, and
BackgroundServiceExceptionBehavior is deliberately left at StopHost so a
genuinely crashed service still stops the host.
Also document the seed-password guard, which is what surfaced this: the image
sets no ASPNETCORE_ENVIRONMENT, so running it outside compose inherits
Production and refuses to start until Seed__AdminPassword is set. That guard
was undocumented outside docker-compose.yml. Adds a README section, a
.env.example (compose points at a .env that had no template), and gitignores
.env — it was tracked, so a compose-created file holding an admin password
would have been committable.
* feat: always seed the default accounts so the app starts out of the box
The admin seed previously failed host startup outside Development when
Seed:AdminPassword was unconfigured, so a plain `docker run` of the image
(which inherits the base image's Production default) refused to boot.
Both seeded accounts now fall back to their compiled-in defaults in every
environment. This also makes the login page's quick-login buttons work as
advertised: they hardcode admin@simplemodule.dev/Admin123! and
user@simplemodule.dev/User123!, and the Users:ShowTestAccounts setting that
renders them defaults to on everywhere — so the demo account previously being
skipped outside Development left a visible button that always failed.
ResolveSeedPassword collapses to "configured value, else default", which makes
SeedPasswordOutcome and SeedConfigurationException dead code; both are removed.
docker-compose no longer requires SEED_ADMIN_PASSWORD (it used ${VAR:?...} on
both api and worker, which blocked startup independently of the C# guard).
These defaults are published in this repository. Startup now logs a warning
outside Development naming the config key to set, and the README, .env.example
and compose comments all call out that a deployment reachable by anyone else
must set Seed:AdminPassword and disable Show Test Accounts. This trades
fail-closed for boots-by-default; it is a deliberate product decision, not an
oversight.
Verified in a container: Production with no seed password now starts, seeds
both accounts with warnings, and login succeeds for both (a wrong password is
still rejected, and /admin/users still redirects anonymous users).
* fix: worker image missing the ASP.NET Core shared framework
Dockerfile.worker built on mcr.microsoft.com/dotnet/runtime:10.0, but the
worker hosts the same modules as the api and every module carries a
<FrameworkReference Include="Microsoft.AspNetCore.App" />. The plain runtime
image does not ship that shared framework, so the worker died on every boot
with "No frameworks were found" and docker compose left it restarting forever.
CI never caught this: it builds only ./Dockerfile, never Dockerfile.worker, and
never starts the stack — and the image builds fine, it only fails at runtime.
This is necessary but not sufficient. With the correct base image the worker
gets further and then fails DI validation, because it registers ASP.NET Core
authorization services while running as a non-web generic host:
AuthorizationPolicyCache cannot resolve EndpointDataSource. That is a separate
pre-existing bug and is not addressed here.
* fix: guard the shutdown drain; address review; drop flaky shutdown tests
Review follow-ups on this branch:
- AuditWriterService and ProgressFlushService drained their channel one last
time AFTER the loop, outside any try. That final flush routinely throws
during a failed startup (the provider is already disposed — observed as
ObjectDisposedException in a real container), which escaped ExecuteAsync as
the very "BackgroundService failed" noise this branch exists to remove, and
skipped LogStopped as well. Both are now guarded and log the error instead.
- AuditRetentionService gained a LogStopped, matching its two sibling services.
- UserSeedService's XML doc referenced a non-existent 'Users:ShowTestAccounts'
config key. The real identifier is the auth.show_test_accounts system setting,
read via ISettingsContracts — it cannot be set by environment variable, so the
old wording pointed operators at a no-op.
- README documents that deployments which previously set only Seed__AdminPassword
will gain a user@simplemodule.dev account seeded with User123! on the next
restart, and corrects "outside Development" to "outside Development or Testing"
to match environment.IsLocalOrTest().
The shutdown regression tests are removed rather than shipped. Three successive
designs — ExecuteTask status, awaiting ExecuteTask, and asserting the service's
own completion log — were all unreliable under parallel load or on CI, and one
of them turned the PR's build red. A test that fails intermittently is worse
than no test: it trains people to ignore CI. The fixes remain verified end to
end in a container, where stopping inside and after the 60s startup-delay
window both exit 0 with zero critical lines.
* docs: correct environment gate in README upgrade note
The upgrade note said the demo user was skipped 'outside Development',
repeating the imprecision corrected four lines above it — the gate is
environment.IsLocalOrTest(), which is Development or Testing.1 parent de00b59 commit bb0837a
12 files changed
Lines changed: 212 additions & 208 deletions
File tree
- modules
- AuditLogs/src/SimpleModule.AuditLogs
- Pipeline
- Retention
- BackgroundJobs/src/SimpleModule.BackgroundJobs/Services
- Users
- src/SimpleModule.Users/Services
- tests/SimpleModule.Users.Tests/Unit
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
| 1 | + | |
| 2 | + | |
| 3 | + | |
| 4 | + | |
| 5 | + | |
| 6 | + | |
| 7 | + | |
| 8 | + | |
| 9 | + | |
| 10 | + | |
| 11 | + | |
| 12 | + | |
| 13 | + | |
| 14 | + | |
| 15 | + | |
| 16 | + | |
| 17 | + | |
| 18 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
451 | 451 | | |
452 | 452 | | |
453 | 453 | | |
| 454 | + | |
| 455 | + | |
| 456 | + | |
| 457 | + | |
| 458 | + | |
| 459 | + | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
94 | 94 | | |
95 | 95 | | |
96 | 96 | | |
97 | | - | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
| 102 | + | |
98 | 103 | | |
99 | 104 | | |
100 | 105 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
39 | 39 | | |
40 | 40 | | |
41 | 41 | | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
| 51 | + | |
| 52 | + | |
| 53 | + | |
| 54 | + | |
| 55 | + | |
| 56 | + | |
| 57 | + | |
| 58 | + | |
| 59 | + | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
| 72 | + | |
| 73 | + | |
| 74 | + | |
| 75 | + | |
| 76 | + | |
| 77 | + | |
| 78 | + | |
| 79 | + | |
42 | 80 | | |
43 | 81 | | |
44 | 82 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
22 | 22 | | |
23 | 23 | | |
24 | 24 | | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
| 25 | + | |
| 26 | + | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
30 | 31 | | |
31 | 32 | | |
32 | 33 | | |
| |||
56 | 57 | | |
57 | 58 | | |
58 | 59 | | |
59 | | - | |
60 | | - | |
61 | | - | |
62 | | - | |
63 | | - | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
64 | 64 | | |
65 | 65 | | |
66 | 66 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
81 | 81 | | |
82 | 82 | | |
83 | 83 | | |
84 | | - | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
85 | 96 | | |
86 | 97 | | |
87 | 98 | | |
| |||
93 | 104 | | |
94 | 105 | | |
95 | 106 | | |
96 | | - | |
| 107 | + | |
| 108 | + | |
| 109 | + | |
| 110 | + | |
| 111 | + | |
| 112 | + | |
| 113 | + | |
| 114 | + | |
| 115 | + | |
| 116 | + | |
| 117 | + | |
| 118 | + | |
| 119 | + | |
| 120 | + | |
| 121 | + | |
97 | 122 | | |
98 | 123 | | |
99 | 124 | | |
| |||
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
16 | 16 | | |
17 | 17 | | |
18 | 18 | | |
19 | | - | |
| 19 | + | |
| 20 | + | |
| 21 | + | |
20 | 22 | | |
21 | | - | |
| 23 | + | |
22 | 24 | | |
23 | | - | |
24 | | - | |
25 | | - | |
26 | | - | |
27 | | - | |
28 | | - | |
29 | | - | |
| 25 | + | |
30 | 26 | | |
31 | | - | |
32 | | - | |
| 27 | + | |
| 28 | + | |
| 29 | + | |
| 30 | + | |
| 31 | + | |
| 32 | + | |
| 33 | + | |
| 34 | + | |
33 | 35 | | |
34 | | - | |
| 36 | + | |
35 | 37 | | |
36 | | - | |
37 | | - | |
38 | | - | |
| 38 | + | |
| 39 | + | |
| 40 | + | |
39 | 41 | | |
40 | | - | |
| 42 | + | |
| 43 | + | |
| 44 | + | |
| 45 | + | |
| 46 | + | |
| 47 | + | |
| 48 | + | |
| 49 | + | |
| 50 | + | |
41 | 51 | | |
| 52 | + | |
| 53 | + | |
42 | 54 | | |
43 | 55 | | |
44 | 56 | | |
| |||
87 | 99 | | |
88 | 100 | | |
89 | 101 | | |
| 102 | + | |
| 103 | + | |
| 104 | + | |
90 | 105 | | |
| Original file line number | Diff line number | Diff line change | |
|---|---|---|---|
| |||
57 | 57 | | |
58 | 58 | | |
59 | 59 | | |
60 | | - | |
| 60 | + | |
| 61 | + | |
| 62 | + | |
| 63 | + | |
| 64 | + | |
| 65 | + | |
| 66 | + | |
| 67 | + | |
| 68 | + | |
| 69 | + | |
| 70 | + | |
| 71 | + | |
61 | 72 | | |
62 | 73 | | |
63 | 74 | | |
| |||
69 | 80 | | |
70 | 81 | | |
71 | 82 | | |
72 | | - | |
| 83 | + | |
| 84 | + | |
| 85 | + | |
| 86 | + | |
| 87 | + | |
| 88 | + | |
| 89 | + | |
| 90 | + | |
| 91 | + | |
| 92 | + | |
| 93 | + | |
| 94 | + | |
| 95 | + | |
| 96 | + | |
| 97 | + | |
| 98 | + | |
| 99 | + | |
| 100 | + | |
| 101 | + | |
73 | 102 | | |
74 | 103 | | |
75 | 104 | | |
| |||
This file was deleted.
This file was deleted.
0 commit comments