Skip to content

Recompile the router when a route is declared after compilation - #2848

Closed
ericproulx wants to merge 1 commit into
masterfrom
fix/recompile-after-route-added
Closed

Recompile the router when a route is declared after compilation#2848
ericproulx wants to merge 1 commit into
masterfrom
fix/recompile-after-route-added

Conversation

@ericproulx

@ericproulx ericproulx commented Aug 1, 2026

Copy link
Copy Markdown
Contributor

Summary

The router is built once, when the API is first compiled, and #compile! memoizes the instance holding it. A route declared after that point was appended to the endpoint list — .routes reported it — but never reached the router, so requests for it answered 404 and recognize_path returned nil. The API's own metadata disagreed with what it served.

Whether it bit depended on what had touched the API first:

sequence before after
get('/one')get('/two') 200 200
api.routes then get('/two') 200 200
a request, then get('/two') 404 200
api.recognize_path('/one'), then get('/two') 404 200
a request, then get('/two'), then explicit api.change! 200 200

In every case .routes already listed /two.

Approach

Call #change! when a route is added. helpers and mount already do this for exactly the same reason — route declaration was the one mutating DSL call that did not, which is why an explicit api.change! was the existing workaround.

The dummy host in routing_spec.rb gains #change!. Grape::DSL::Routing already required it of its host: #mount calls it bare too, just from a branch that dummy never reaches.

Perf

The normal shape — declare every route, then serve — is unaffected. #change! is a single ivar assignment and compilation is lazy, so the calls made while a class body is evaluated cost nothing:

2000 routes
before 1.247s
after 1.242s

Serving 200 requests against a fully-declared API takes 0.075s, unchanged.

Interleaving declaration with serving now costs O(n²), because each declaration discards the compiled router and the next request rebuilds every endpoint so far:

declare+request, alternating total per route
n=25 0.039s 1.56 ms
n=50 0.112s 2.25 ms
n=100 0.433s 4.33 ms
n=200 1.758s 8.79 ms

Worth stating plainly rather than burying: on master the same loop takes 0.014s — but only because it is not doing the work. Every one of those requests answers 404; that is the bug. The cost is the price of the routes actually being served, and it is paid only by a pattern that previously did not function.

If that pattern matters enough, the follow-up is an incremental router append rather than a full rebuild on invalidation — a substantially larger change to Grape::Router, deliberately not attempted here.

Backward compatibility

No UPGRADING entry. A route that previously 404'd now answers; nothing could have relied on a declared route being unreachable. APIs defined entirely before first use — effectively all of them — are unaffected, since the route table is identical by the time anything compiles.

Longstanding, not a regression: identical in 3.3.4.

Test plan

  • 4 new examples in api_spec.rb covering a route added after a request and after recognize_path, its visibility to recognize_path, and a guard that earlier routes keep serving; verified 3 fail without the lib/ change.
  • Full RSpec suite passes locally (2547 examples, 0 failures).
  • RuboCop clean.
  • CI green.

🤖 Generated with Claude Code

The router is built once, when the API is first compiled, and #compile!
memoizes the instance holding it. A route declared after that point was
appended to the endpoint list -- .routes reported it -- but never reached the
router, so requests for it answered 404 and recognize_path returned nil. The
API's own metadata disagreed with what it served.

Whether it bit depended on what had touched the API first:

    get('/one'); get('/two')                      # /two served
    api.routes; get('/two')                       # /two served
    Rack::Test.new(api).get('/one'); get('/two')  # /two -> 404
    api.recognize_path('/one'); get('/two')       # /two -> 404

`helpers` and `mount` already call #change! for exactly this reason -- route
declaration was the one mutating DSL call that did not.

The dummy host in routing_spec gains #change!, which Grape::DSL::Routing
already required of it: #mount calls it too, just from a branch that dummy
never reaches.

No boot cost: #change! is an ivar assignment, and compilation is lazy, so
defining 2000 routes takes the same 1.24s either way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@ericproulx
ericproulx force-pushed the fix/recompile-after-route-added branch from ceb2cc1 to e619622 Compare August 1, 2026 12:16
@github-actions

github-actions Bot commented Aug 1, 2026

Copy link
Copy Markdown

Danger Report

No issues found.

View run

@ericproulx

Copy link
Copy Markdown
Contributor Author

Closing: the scenario this fixes is not one Grape supports.

Checking which shapes actually break on master:

shape master
routes declared after mount 200
routes declared after helpers 200
routes declared after reading .routes 200
routes declared after an explicit compile! 404
routes declared after serving a request 404

Every ordinary class-body arrangement already works. The only two that break require the API to have been compiled first — that is, mutating a live, already-serving API, which is undocumented and not thread-safe (compile!'s double-checked locking is unsynchronised).

That also matches what #change! is for. It is reachable from exactly two places, helpers (via include_all_in_scope) and mount, both class-body DSL calls: its job is keeping the compiled instance coherent while the class body is still being evaluated, not supporting live route addition. The README documents reloading — grape-reload, Zeitwerk in development — which redefines classes rather than mutating compiled ones.

So this fixed only an unsupported case, and charged O(n²) for it: each declaration discarded the compiled router, so interleaving declaration with serving went 1.56 → 8.79 ms/route between n=25 and n=200.

If the .routes-versus-routing disagreement is ever worth addressing, the honest fix is making runtime mutation fail loudly, or documenting that an API is fixed once compiled — a design decision rather than a bug fix.

Dropped from #2850 as well.

@ericproulx ericproulx closed this Aug 1, 2026
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