Lint every response of an API once under lint! - #2960
Open
ericproulx wants to merge 1 commit into
Open
ericproulx wants to merge 1 commit into
ericproulx wants to merge 1 commit into
Conversation
ericproulx
force-pushed
the
feat/lint-router
branch
from
September 18, 2026 14:43
2735bd2 to
0a7bdaa
Compare
Danger ReportNo issues found. |
ericproulx
force-pushed
the
feat/lint-router
branch
2 times, most recently
from
September 18, 2026 15:09
93e1f0f to
a24a5fe
Compare
ericproulx
added this pull request to stack #2962
September 18, 2026 20:19
`lint!` and Grape.config.lint put Rack::Lint in each endpoint's stack, so two kinds of response never met it: the 404 the router answers for a path nothing matches, which no endpoint builds, and a Rack app mounted with `mount`, which the router calls directly instead of through a stack. That is how a HEAD request for an unknown path answered with a body (#2958) while the suite, which lints every API, stayed green. The per-endpoint Rack::Lint is gone. A linted API instead wraps its router in a single Rack::Lint, which checks every response the API gives -- endpoints, errors, the router's own 404, mounted Rack apps -- exactly once, and after Rack::Head, so a HEAD response is checked with its body stripped. Keeping the endpoint Lints and adding one around the router would nest two, and nested Rack::Lints cannot survive a middleware that calls `to_ary` on the body -- the inner one closes it, the outer one then iterates it -- which Rack::ETag does in any Rails app mounting the API. `lint!` is therefore read from the top of the API that is served, as the README describes it ("at the API level"). Declared inside a namespace, or in an API mounted into another one, it no longer does anything; README and UPGRADING say so. The suite's own lint then flagged two fixtures: `API.call({})`, an env Rack::Lint rejects, and a mounted Rack app returning a frozen headers Hash, which Rack 3 forbids. The first now passes a real env; the second an unfrozen Hash the spec still checks is not written into. Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
ericproulx
force-pushed
the
feat/lint-router
branch
from
September 18, 2026 20:26
a24a5fe to
e563b2e
Compare
dblock
approved these changes
Sep 19, 2026
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.
Summary
lint!andGrape.config.lintputRack::Lintin each endpoint's stack. Two kinds of response never meet it:mount, which the router calls directly, not through a stack.That's how #2958's HEAD-with-a-body 404 got past a suite that lints every API. It also means a mounted Rack app can break the Rack SPEC under
lint!without anything noticing:This PR removes the per-endpoint
Rack::Lint. A linted API wraps its router in a singleRack::Lint, which checks every response the API gives exactly once: endpoints, errors, the router's own 404 and mounted Rack apps. It runs afterRack::Head, so HEAD responses are checked with their body already stripped.Why one Lint, not one more
Keeping the endpoint Lints and adding one around the router would nest two. Nested
Rack::Lints break when a middleware callsto_aryon the body: the inner one closes the body insideto_ary, then the outer one iterates it (Response body is already closed).Rack::ETagdoes exactly that in any Rails app that mounts the API, andspec/integration/rails/mounting_spec.rbfailed that way on my first attempt.Scope of
lint!(behaviour change)lint!is now read from the top of the API being served, which matches what the README documents ("at the API level"). Declared inside a namespace, or in an API mounted into another one, it no longer does anything. A root-levellint!covers mounted APIs' endpoints, andGrape.config.lintcovers everything as before. README and UPGRADING both describe this.lint!is declaredGrape.config.lintFixtures
With every response linted, the suite's own lint flagged two fixtures that break the SPEC:
API.call({})inapi_spec: Lint rejects the empty env. It now passesRack::MockRequest.env_for('/').instance_specreturning a frozen headers Hash, which Rack 3 forbids. It now returns an unfrozen Hash, which the spec still checks isn't written into.UPGRADING notes that suites using
lint!may start raising in cases like these.Test plan
.lint!examples: an endpoint's bad status, an unmatched request with a bad env, a mounted Rack app, and an endpoint of a mounted Grape API all raise; HEAD to an unmatched path passes. Without the Lint around the router, the four raising examples fail.spec/integrationpasses withgemfiles/rails_8_0.gemfile(the Rails mounting spec) andgemfiles/grape_entity.gemfile.🤖 Generated with Claude Code