Flower ServerApp templates should relay their own exceptions to the hub - #1007
Open
atriaybagur wants to merge 2 commits into
Open
Flower ServerApp templates should relay their own exceptions to the hub#1007atriaybagur wants to merge 2 commits into
atriaybagur wants to merge 2 commits into
Conversation
…#1006) Both Flower templates' main() bodies now run inside a relay shell: any exception escaping the run is reported to the hub — the full traceback via send_handled_exception (server-side, no trust boundary crossed) and the model settled to ERROR — before being re-raised so Flower still records finished:failed and the #1003 failed-job sweep stays consistent as the backstop. Each relay step is guarded separately so an unreachable hub or a non-UUID tutorial model id cannot mask the original failure. The researcher-supplied models.py import moves from module scope into the guarded body: a broken models.py — the most likely researcher error — now reports its traceback to the activity feed instead of killing the ServerApp before it can say anything (the FLIP#1001 canonical case, previously visible only to the hub's log-tail poll). The relay helper is deliberately duplicated per template rather than added to flip.flower: templates ship in the flip-api image while flip-utils ships in the FL images, so a new flip.flower symbol would die with ImportError at module scope on any FL image older than the template — the exact failure mode this change exists to end. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
6 tasks
…ates check_tutorial_sync.sh pins these as byte-identical copies of the fl-apps templates (flwr build excludes symlinks from the FAB, so each tutorial keeps a real copy). The relay-shell change therefore lands in all three copies: xray_classification and 3d_spleen_segmentation from flower/standard, 3d_spleen_segmentation_evaluation from flower/evaluation. In the tutorials' standalone submit path FLIP() resolves to the dev no-op implementation, so the relay just logs there — behaviourally inert outside a hub deployment. Signed-off-by: at24_bioeng625-pc <alexandre.triay_bagur@kcl.ac.uk>
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
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.
Closes #1006. The in-run half of #1001 (whose external half — the hub's failed-job poll — is #1003).
What changes
Both Flower templates (
fl-apps/flower/{standard,evaluation}/app/server_app.py) get a relay shell around their run body:main()becomes a thin wrapper:try: _run(...) except Exception: _relay_failure(...); raise._relay_failuresends the fulltraceback.format_exc()to the hub viaflip.send_handled_exception(asuccess=falseactivity-feed row; the ServerApp runs on the Central Hub, so no trust boundary is crossed — unlike ClientApp exceptions, where the recorded design decision is type+message only) and settles the model toERROR. Each step is guarded separately, so an unreachable hub or a non-UUID model id (tutorial/simulator contexts, whereFLIP()resolves to the dev no-op anyway) cannot mask the original failure.finished:failedand the fix: surface FL runs that fail after submission (#1001) #1003 sweep stays consistent — its post-poll model-status re-read makes the double sighting a no-op.models.pyimport moves from module scope into the guarded body: a brokenmodels.py— the most likely researcher error — now lands its traceback on the activity feed instead of killing the ServerApp before it can say anything (the [Bug]: a Flower run that fails after submission is invisible — model sits at INITIATED with no error #1001 canonical case, previously visible only through the sweep's captured log tail).Existing in-body error paths are unchanged:
parse_best_model_run_configfailures now additionally get their traceback relayed by the shell (they raise); save/upload failures stillreturnafter their ownERRORand never reach the shell. A secondupdate_status(ERROR)for the parse path is a no-op on the hub (same-status transition).Merge order relative to #992
Deliberately sequenced after #992 (
flower-min-clients-from-trust-count): both PRs touch the same five files (the two templateserver_app.pyand their three CI-pinned tutorial mirrors). #992 is load-bearing for more than itself — it fixes themin_nodes=2single-trust hang that blocks Flower on a second dev instance — while this PR is self-contained hardening that rebases trivially (the relay wrapsmain(); #992 changes strategy construction inside the body). When #992 lands, this branch gets rebased, the five files re-resolved, andcheck_tutorial_syncre-run.Why the helper is duplicated per template, not added to
flip.flowerTemplates ship in the flip-api image;
flip-utilsships in the FL images — they deploy separately. A template importing a newflip.flowersymbol dies withImportErrorat module scope on any FL image older than the template: the exact failure mode this change exists to end. Self-containment costs ~20 duplicated lines and removes the skew entirely.Scope
bundle_flower_applicationskips user files whose names collide with template files, so the template'sserver_app.pyalways wins.flip_server_event_handleralready relays terminal failures (FATAL_SYSTEM_ERROR,END_RUN).fl-tutorials/flower/*/app/server_app.py) are resynced byte-identically —scripts/check_tutorial_sync.shpins them as exact copies of the templates (CI enforces it; the first push here went red on exactly that). In the tutorials' standalonemake submitpathFLIP()resolves to the dev no-op implementation, so the relay just logs there — behaviourally inert outside a hub deployment.Verified
fl-apps/has no test harness, per the issue):server_appmodules import with noapp/models.pypresent — proving the module-scope decoupling (pre-change this import was the module-scope death);_runis relayed (send_handled_exceptioncalled once with the traceback,client_name=None, the run-config model id;update_status(<id>, ERROR)) and re-raised;send_handled_exceptionraisingValueErroron the non-UUID tutorial id) still settles status and re-raises the original. 6/6 passed.ruff check(0.14.7, the template pyprojects' own config) clean on bothapp/trees.required_files.jsonchange (no files added/removed).bash scripts/check_tutorial_sync.sh— all copies in sync after the resync commit.NOT verified
maindirectly with a mockedflipand a stubbed context. The standard template is exercised end-to-end bymake e2e_smoke FL_BACKEND=flowerwhen a stack is available.Acceptance Criteria
Imported from issue #1006
main()— in either template — moves its model toERRORand writes the traceback tofl_logsviasend_handled_exception, without waiting for the reconcile sweep.models.py(ImportError/SyntaxError onget_modelimport) is reported the same way rather than dying at ServerApp module scope.finished:failedand the fix: surface FL runs that fail after submission (#1001) #1003 sweep stays consistent.ERRORtransitions with different meanings.flip-model-iddoes not crash the handler — the relay degrades gracefully.