Repository navigation
perf: skip the WebSocket handshake probe without an Upgrade header - #55
Merged
Merged
Conversation
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.
The middleware chose the child's scope by running
web.WebSocketResponse().can_prepare(request)on every request. For a plain HTTP request that probe builds aWebSocketResponse, then builds, raises and catches anHTTPBadRequest, because the first check in aiohttp's_handshakeis thatUpgradeequalswebsocket.The probe now runs only when the request has an
Upgradeheader. Without one the result was already "not a WebSocket", so the scope chosen for every request is the same as before. Any request that does carry the header is still decided by aiohttp's own check, as ADR-0002 requires. The ADR gets one sentence about the skip.Measured by calling the middleware directly with a mocked GET request, median of 7 runs x 50k calls, run twice:
The new test wraps
web.WebSocketResponsein a mock and asserts that a plain GET never constructs one. It fails on main withassert 1 == 0. The existing WebSocket tests cover the upgrade path.