Skip to content

fix(routes): register FastAPI and Starlette WebSocket decorators - #2258

Open
DeusData wants to merge 1 commit into
mainfrom
distill/1245-websocket-route-suffixes
Open

DeusData wants to merge 1 commit into
mainfrom
distill/1245-websocket-route-suffixes

Conversation

@DeusData

Copy link
Copy Markdown
Owner

Distilled from #1245 by @ahundt (a slice of 1ac3c13), carried with Co-authored-by. #1245 stays open until its distills land.

The bug

The route-registration suffix table knew .add_api_websocket_route but not the forms FastAPI and Starlette actually use — @router.websocket("/ws") and @app.websocket_route("/events") — so a WebSocket endpoint produced no Route node on either resolver path.

The fix

Two table entries, both recorded as ANY like the existing WebSocket entry (a socket upgrade is not an HTTP verb). Both are needed: matching is a pure suffix compare, and .route does not match …websocket_route, whose tail is _route.

Upstream also changed pass_calls.c, pass_parallel.c and pipeline_internal.h. None of that is taken — main already carries the #952 sequential unresolved-callee route fallback its July base lacked, so the table is the only missing piece.

RED → GREEN

Each form recorded on its own (the harness stops at the first failing assert, so they were swapped and re-run):

FAIL tests/test_pipeline.c:6163: count_nodes_named(s, project, "/ws") (0) not >= 1 (1)
FAIL tests/test_pipeline.c:6163: count_nodes_named(s, project, "/events") (0) not >= 1 (1)

The @app.get("/health") control passed before and after.

After: pipeline 282 passed; pipeline parallel extraction edge_types_probe route_canon infrascan 779 passed, 0 failed.

The test drives the sequential resolver (one file). The parallel resolver reads the same table via cbm_service_pattern_route_method and its suite is green; a CBM_WORKERS=4 twin is a small follow-up.

Distilled from #1245 by Andrew Hundt (a slice of 1ac3c13).

The route-registration suffix table knew .add_api_websocket_route but
not the decorator forms FastAPI and Starlette actually use:

    @router.websocket("/ws")
    @app.websocket_route("/events")

so a WebSocket endpoint produced no Route node on either resolver path
and was invisible to the graph. Two entries join the table, both
recorded as ANY like the existing WebSocket entry -- a socket upgrade is
not an HTTP verb. Both are needed: matching is a pure suffix compare and
.route does not match ...websocket_route, whose tail is _route.

Upstream also changed pass_calls.c, pass_parallel.c and
pipeline_internal.h. None of that is taken: main already carries the #952
sequential unresolved-callee route fallback its July base lacked, so the
table entries are the only missing piece.

RED before the fix, each form recorded on its own (the harness stops at
the first failing assert, so the two were swapped and re-run):
  pipeline_python_websocket_decorators_register_routes
      FAIL tests/test_pipeline.c:6163:
      count_nodes_named(s, project, "/ws") (0) not >= 1 (1)
      FAIL tests/test_pipeline.c:6163:
      count_nodes_named(s, project, "/events") (0) not >= 1 (1)
The @app.get("/health") control above those lines passed before and
after.

GREEN after: pipeline 282 passed; pipeline parallel extraction
edge_types_probe route_canon infrascan 779 passed, 0 failed.

The test drives the sequential resolver (one file). The parallel
resolver reads the same table through cbm_service_pattern_route_method
and its suite is green; a CBM_WORKERS=4 twin of this test is a small
follow-up.

Co-authored-by: Andrew Hundt <ATHundt@gmail.com>
Signed-off-by: Martin Vogel <martin.vogel.tech@gmail.com>
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.

1 participant