Skip to content

One app's ImportError takes the whole gateway down — mount a 500 stub for that app instead #37

Description

@thorwhalen

Decision item §9.3 of the Cake-and-Eat-It review (2026-08-16), whose default is yes and which the review says "deserves its own ADR". Boot is all-or-nothing for mode="asgi" apps: one un-importable app kills every gunicorn worker, the master shuts down, and every app on the platform 502s. The proposal is to change the failure policy from loud and global to loud and local — on import failure, mount a 500 stub for that app and scream in /_meta, in enlace doctor, and in the logs.

The principle "never silently swallow an import error" is preserved in full. Only the blast radius changes. The distinction that makes it safe: a stub is not a degraded success. It has to be as loud as a crash-loop, just not as wide.

The chain, in code — two sites, not one

Discovery raises first:

  • enlace/discover.py:110-113 — _detect_app_type(...) is called with no guard.
  • enlace/discover.py:226, docstring at :235-238 — "Raises ImportError … This is intentionally NOT caught — silently swallowing import errors is an anti-pattern."

Compose raises again, independently:

  • enlace/compose.py:87-91 — the mount loop calls strategy.make_asgi(app_config, config) with no try/except.
  • enlace/strategies.py:284-291 — AsgiStrategy.make_asgi → enlace/compose.py:678 _load_sub_app → enlace/compose.py:727 _import_app_module → bare importlib.import_module.

So both sites need handling. The discovery site is handled by the prerequisite issue in this repo, #36 (the on_import_error seam on discover_apps); this issue owns the compose site and the policy decision.

Scope is mode="asgi" only: process, external and static apps skip Python introspection entirely (enlace/discover.py:83, enlace/strategies.py:112 vs :277).

It is not hypothetical — it happened two days after the review

thorwhalen/tw_platform#123 (2026-08-18; private repo, linked for those with access): a single-app deploy of an app with no Python in it at all fast-forwarded an editable external whose new code imported a symbol that the installed non-editable copy of its dependency did not have. Backend restart → ImportError: cannot import name … → gunicorn Worker failed to boot → master shut down → 502 on every gated app, live, before the smoke gate could run.

Loud-and-local turns that into: one app returning 5xx, the other ~30 serving, a red enlace doctor, a red /_meta. That issue separately proposes a pre-restart import check on the deploy side; the two are complementary — that check removes the restart, this issue removes the blast radius.

The change

  1. Carry the failure on AppConfig (enlace/base.py:104; the model is extra="allow" at :99, so this is additive) — done by the prerequisite issue (One un-importable app kills enlace doctor before it runs a single check #36).
  2. enlace/strategies.py:284-291 — AsgiStrategy.make_asgi returns a stub ASGI app for a degraded config: every route under the app's prefix returns 5xx with a short body naming the app and the exception class. No traceback to unauthenticated callers.
  3. Surface it in three places:
    • enlace/compose.py:275 _add_meta_routes / :305 _register_app_meta — the app's /_meta and the platform /_meta report the degraded state.
    • enlace/doctor.py — the FAIL check added by the prerequisite issue (One un-importable app kills enlace doctor before it runs a single check #36) now also covers apps stubbed at compose time.
    • enlace/compose.py:526 _add_apps_listing_route — the launcher shows the app as broken, not launchable.

Precedent for degrade-don't-crash already exists in this codebase: enlace/manifest.py:112 — "Don't let a corrupt manifest crash the server — log and degrade" — and enlace/compose.py:448 — "a flaky store must never break the listing". Loud-and-local applies the same judgement one layer up.

Open questions the ADR must answer

  1. Every asgi app, or only a declared shared tier? A tier-scoped policy keeps the strictest behaviour available to anyone who wants "refuse to boot" — including an app with paying users that would rather be absent than serve a 500. The tiering work is on the deploy-platform side (thorwhalen/tw_platform#148); if the answer is tier-scoped, this issue needs the tier to exist first, which is a real sequencing cost.
  2. Route conflicts, currently also fatal at boot. A conflict is a config error naming two known apps; stubbing both may be right, or it may hide a genuine ambiguity about which app owns a prefix.
  3. Lifespan failures, which happen after import inside the shared AsyncExitStack (enlace/compose.py:94 cascade_lifespan, stack at :101). Same treatment, or different? The failure is later, the state is dirtier, and partial teardown is involved.
  4. Per-app opt-out — is fail_hard = true in app.toml part of v1, or deferred?

The test this breaks — needs an explicit OK before it is touched

enlace/tests/test_discover.py:129 test_discover_import_error_propagates asserts precisely the behaviour being changed:

with pytest.raises(ModuleNotFoundError, match="nonexistent_package_xyz"):
    discoverer.discover(tmp_apps_dir)

The prerequisite issue (#36) keeps it green by defaulting the new seam to raise. This issue is where the default flips, so this test becomes test_discover_import_error_degrades_one_app. Do not change it without confirming the policy decision first.

This is not a reversal of #1 — and the review's citation of #1 is wrong

The review attributes the "never swallow" rationale to #1. It is not there. #1's body contains no such argument; the rationale lives in the enlace/discover.py:235-238 docstring. Two independent audit lanes checked this and agreed.

What #1 actually says, verbatim, is that the shared failure domain is a problem it wants solved — its numbered item 2, "Shared failure domain. All apps run in one process — if one app crashes with a segfault or C-extension error, every app goes down", and its item 5, "Discovery requires Python import. _detect_app_type() imports every module at discovery time." #1 was closed on 2026-04-14 by PR #2 (66165b4) on shipping the four modes, whose remedy for item 2 was "use process mode". Item 5 is still exactly true at enlace/discover.py:110-113.

Loud-and-local is therefore a continuation of #1's stated goals, not a revision of them. The ADR should record the decision on its own merits, not as a rebuttal of #1.

Acceptance criteria

  • An app with a deliberate import failure is mounted as a 500 stub; every other app serves normally; the gateway boots.
  • Requests under the broken app's prefix return 5xx naming the app and the exception class; no traceback in the response body.
  • enlace doctor exits non-zero — FAIL, not inconclusive, not pass — while any stub is mounted, and names the app and the exception.
  • Platform /_meta and the app's /_meta report the degraded state; /_apps does not present the app as launchable.
  • A monitored signal changes the moment a stub is mounted. Replaying that outage's shape end to end produces one broken app plus an alert, not a site-wide 502 plus silence. A stub nobody is told about is a worse outcome than the crash-loop it replaced.
  • Tests cover: one broken app among several; the broken app being the landing app; a broken process-mode app is unaffected; and the property "no stub is ever silent" (every stub implies a FAIL check and a /_meta entry).
  • An ADR under misc/docs/decisions/ (a new directory in this repo — this repo has none yet; the deploy platform's ADR template is a good shape to copy) records loud-and-local vs loud-and-global, answers the four open questions above, and supersedes the enlace/discover.py:235-238 docstring rationale.
  • Released to PyPI. The production platform installs enlace from PyPI (0.1.28 is current), so nothing reaches production until a release.

Publishing note

This changes the boot behaviour of a published package that has dependents beyond this platform. Per the owner's policy it needs the ADR above and an independent adversarial review before landing, not just a normal PR review. The blast radius of getting this wrong is the inverse of the bug: a platform that boots looking healthy while serving 500s.

Merge note

Two audit lanes filed this independently; this is the merge. The code chain, the outage evidence, the in-repo degrade-don't-crash precedent and the #1 correction come from the deep-code lane; the open ADR questions, the "a stub is not a degraded success" framing and the monitored-signal acceptance criterion come from the tracker-coverage lane. Where they disagreed on effort (M vs L), the larger estimate wins: this is three surfaces, a policy ADR, an adversarial review and a PyPI release, not a patch.

Related: thorwhalen/tw_platform#123 (the live outage), the tiered-isolation issue thorwhalen/tw_platform#148 (this is the shared tier's prerequisite), and the systemd-generation issue in this repo, #38 (the isolated tier's).

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    design discussionTo discuss design options, pros and cons, etc.enhancementNew feature or request

    Type

    No type

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions