Add METH_QUERY and treat QUERY as an idempotent method - #13301
Conversation
QUERY is defined as safe and idempotent by RFC 10008, so the client can replay it on a keep-alive connection the server closed underneath us, the same as GET or PUT. The constant is exported alongside the other METH_* names. METH_QUERY stays out of METH_ALL on purpose: RouteDef.register dispatches every METH_ALL member through getattr(router, "add_" + method.lower()), and UrlDispatcher has no add_query().
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #13301 +/- ##
=======================================
Coverage 98.98% 98.98%
=======================================
Files 132 132
Lines 49023 49070 +47
Branches 2551 2553 +2
=======================================
+ Hits 48526 48573 +47
Misses 373 373
Partials 124 124
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. |
Merging this PR will not alter performance
Comparing Footnotes
|
Backport to 3.15: 💔 cherry-picking failed — conflicts found❌ Failed to cleanly apply 58bae08 on top of patchback/backports/3.15/58bae08b7e4831c6c184fe22233bfc19941c700b/pr-13301 Backporting merged PR #13301 into master
🤖 @patchback |
What do these changes do?
Follow-up to #13160, as requested in #13160 (comment).
#13174 fixed the parser half of that issue, so
QUERYnow arrives intact instead of<unknown>. The two remaining pieces (originally part of the closed #13167) never landed:aiohttp/hdrs.pygainsMETH_QUERY, alongside the otherMETH_*constants.aiohttp/client.pyadds"QUERY"toIDEMPOTENT_METHODS, so aQUERYrequest is replayed when the server closes a keep-alive connection underneath it, the same asGETorPUT. RFC 10008 defines the method as both safe and idempotent (https://www.rfc-editor.org/info/rfc10008/#section-1-12), which is exactly the property that set encodes.METH_QUERYis deliberately not added toMETH_ALL.RouteDef.register()dispatches everyMETH_ALLmember throughgetattr(router, "add_" + method.lower()), andUrlDispatcherhas noadd_query(), so adding it there would turnweb.route("QUERY", ...)passed toadd_routes()into anAttributeError. (METH_CONNECTandMETH_TRACEare already inMETH_ALLwithout matchingadd_*methods, so that path is broken for them today — out of scope here.)Are there changes in behavior for the user?
Yes, one: a
QUERYrequest that hits a keep-alive connection the server has just closed is now retried once instead of raisingServerDisconnectedError. That is the documented behavior for safe, idempotent methods, andQUERYqualifies. Everything else is additive — a new public constant.Is it a substantial burden for the maintainers to support this?
No. It is one constant and one set member, with no new code paths; the retry logic itself is untouched.
Related issue number
Follows up #13160 (fixed for the parser by #13174). Reuses the
hdrs.py/client.pyhunks from the closed #13167, with the RFC URL amended to the form suggested in review there.Checklist
hdrsconstants are not individually documented and the retry behavior is described genericallyCONTRIBUTORS.txt— N/A, already listedCHANGES/folderTest evidence
New test
test_retry_persistent_connection_query_methodsits alongside the existingtest_retry_persistent_connection_lowercase_method, and follows the same shape: the handler force-closes the connection on the first request, and the test asserts the request is retried.Passes with both parsers:
Confirmed the test fails without the production change (reverting
IDEMPOTENT_METHODSto its previous value makes it error with the connection dropped, no retry), so it is not vacuous.Wider run, no regressions:
black,isortandmypyare clean on the touched files.