Skip to content

fix(server): preserve non-404 stage Airflow failures - #378

Merged
kstonekuan merged 1 commit into
Hebbian-Robotics:mainfrom
victorwon2001:fix/issue-312
Sep 3, 2026
Merged

fix(server): preserve non-404 stage Airflow failures#378
kstonekuan merged 1 commit into
Hebbian-Robotics:mainfrom
victorwon2001:fix/issue-312

Conversation

@victorwon2001

Copy link
Copy Markdown
Contributor

Summary

  • Preserve the existing HTTP 404 degradation for unregistered stage DAGs and expired stage task details.
  • Return the existing browser-safe 502 response for all other stage AirflowClientError failures, including errors without an HTTP status.
  • Add route-level regression coverage for all three previously suppressed call sites and for remote failure sanitization.

Fixes #312

Why

Stage run surfaces previously converted authentication, rate-limit, upstream 5xx, and status-less Airflow failures into empty lanes or recent-run strips. Restricting degradation to 404 keeps the optional-stage behavior while surfacing real upstream failures through the same sanitized 502 path already used for master DAG failures.

Validation

  • uv run ruff check --fix — passed.
  • uv run ruff format — passed; 208 files unchanged.
  • uv run ty check — passed.
  • uv run pytest -q packages/hflow-server/tests/test_server_runtime.py packages/hflow-server/tests/test_server_run_graph.py — 35 passed.
  • uv run pytest -q — 1440 passed, 6 skipped after installing the documented ffmpeg/ffprobe prerequisite.

Checklist

  • I added or updated outcome-focused tests for changed business logic.
  • I updated documentation for changed behavior, flags, formats, or requirements.
  • I ran uv run ruff check --fix, uv run ruff format, and uv run ty check.
  • I ran the relevant pytest suite.
  • I did not add recordings, generated media, credentials, private URLs, or runtime artifacts.
  • I preserved stored-data compatibility or documented an explicit version change.

@kstonekuan kstonekuan left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, merging. Welcome, and this is unusually complete for a first PR.

All three suppressed call sites are held in both directions, which is the property that matters: the point of #312 was to stop swallowing real failures without losing the degradation the suppression existed for.

call site                non-404 must refuse                              404 must still degrade
stage task instances     ..._stage_task_failure_without_status_maps_to_502  ..._tolerates_missing_task_detail_for_a_matched_stage_run
stage run listing        ..._remote_stage_listing_failure_is_a_sanitized_502 ..._tolerates_unregistered_stage_sub_dags
recent stage runs        test_runs_maps_stage_rate_limit_to_502             test_runs_tolerates_an_unregistered_stage_dag

Each of those six is the only failure when its side is reverted. Two of the three degradation tests are pre-existing, which is the better kind of evidence: the tests that defined the old behaviour are what constrain the new one.

DoD 4 is specifically covered rather than incidentally. error.status == 404 is False for None, so a status-less error takes the 502 path, and treating None as a 404 fails a test that exists for exactly that:

status-less errors treated as 404  ->  test_run_graph_stage_task_failure_without_status_maps_to_502

The sanitization test is the one I would have written. Seeding the environment with a real-looking internal URL and token, putting both plus a traceback into the upstream error message, and then asserting the response text contains none of them:

assert "airflow.internal.corp" not in response.text
assert "minted-token" not in response.text
assert "Traceback" not in response.text

That is DoD 5 as a test rather than an inspection, and it checks the whole response rather than just the detail field, so a leak through some other key would still fail it.

Threading source into stage_task_instances rather than reaching for a module-level value keeps the refusal using the same RuntimeSource the rest of the route already resolved, so the local and remote details stay distinguishable.

Gate clean with current main merged in: ruff check, ruff format --check, ty check, 1440 passed / 6 skipped.

Closes #312. If you want another, #311 covers the pagination on the other side of the same client and is close to this in shape, though there is an open PR against it right now, so #310's neighbours in the advanced list are the safer pick: https://github.com/Hebbian-Robotics/hflow/issues?q=is%3Aissue+is%3Aopen+label%3Aadvanced

@kstonekuan
kstonekuan merged commit ca0fe80 into Hebbian-Robotics:main Sep 3, 2026
9 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.

Preserve non-404 Airflow failures on stage run surfaces

2 participants