Repository navigation
fix(responses): a canned reply outside 2xx reads error on Tendermint RPC, and a gRPC provider refuses a canned body - #170
Merged
Conversation
…RPC, and a gRPC provider refuses a canned body A review of the canned reply with an error status found three gaps. Tendermint RPC serves a canned body as the result of the reply, with its status or http_status. Its listener had no label rule of its own, so a canned 503 was recorded as success. It now records a reply outside 2xx as error, as REST and GraphQL do. The gRPC listener never reads a canned body. A body on a provider with only gRPC endpoints was stored and changed no reply; with the error status now accepted, that reached more scenarios. The control API now refuses it, as it refuses the other settings a provider cannot apply, and names the keys a gRPC provider reads. The tests of the label used bodies that agreed with their status, so a rule that also read the body, or called only 200 a success, passed them. A canned 503 with no error in its body, and a canned 299 with an error, now pin the rule. Each of three broken label rules fails one of them. Text that the canned error status made untrue is corrected: the test docstrings and one failure message, the heading in jsonrpc.py, the control API comment, and the user guide with its GraphQL table.
… a body no endpoint of a gRPC provider serves A review of this branch found three gaps. Tendermint RPC reads http_status as the status of a canned reply, as it reads status, but only status was validated. A text "503" fell back to HTTP 200 and changed no reply, and a 302 was sent. Both keys now get the same rule: an int from 200 to 299 or from 400 to 599. The gRPC rule refused a body only when every endpoint was gRPC. A provider with a REST endpoint beside its gRPC one accepted a body under a method name, and no endpoint served it: REST reads a [verb, template] pair. The rule now refuses such a body on a provider that has a gRPC endpoint and no JSON-RPC, GraphQL or Tendermint RPC endpoint. The Tendermint label covers every reply outside 2xx, also one whose status a provider-wide http_status sets. A test now pins that, a 302 included: a rule that called only 400 and up an error fails it. The user guide states the rule for both keys, the wider Tendermint label, the narrower gRPC refusal, and that a GraphQL canned reply needs no status.
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.
Why
A review of PR #169 (a canned reply can carry an error status, and the history records it as error) found gaps after it merged, and a review of this branch found three more:
success. The chain wraps a canned body as theresultof the reply, and the Tendermint listener had no label rule of its own, so the label read only the body.body. A body under a method name on a gRPC provider was stored and changed no reply. feat(responses): a canned reply can carry an error status, and the history records it as error #169 turned the refusal of an error status there into silent acceptance.http_statusas the status of a canned reply, as it readsstatus, but onlystatuswas validated. A text"503"fell back to HTTP 200, and a 302 was sent.What changed
provider_simulator/listeners/tendermint.py: every reply outside 2xx readserrorin the history, as on REST and GraphQL: a canned reply, and a reply whose status a provider-widehttp_statussets. Inside 2xx the label is unchanged.provider_simulator/control_api.py:statusandhttp_statusbeside a cannedbodyboth must be an int from 200 to 299 or from 400 to 599. The message forstatuskeeps the wording of feat(responses): a canned reply can carry an error status, and the history records it as error #169._a_canned_body_no_endpoint_can_serverefuses abodyunder a method name on a provider that has a gRPC endpoint and no JSON-RPC, GraphQL or Tendermint RPC endpoint, with HTTP 400 that names the entry, the endpoints of the provider, and the keys a gRPC endpoint reads (result,error,error_stub). A REST[verb, template]entry on a provider with a REST endpoint is accepted. Like the two corruption rules beside it, it reads the endpoints of the provider and no filter.tests/test_listener_tendermint.py: a canned 503 set withstatusor withhttp_statusreadserror, a canned 299 readssuccess, and a provider-widehttp_statusof 302 or 503 readserrorwhile 200 readssuccess.tests/test_control_api.py: a canned body onlava-sim-grpc:1, with status 200 or 503, is refused, and so is a body under a method name on a provider with a gRPC and a REST endpoint. A cannedhttp_statusof 302, 100,"503"ortrueis refused. The controls: aresulton the gRPC provider, a canned body on JSON-RPC and on Tendermint RPC, a cannedhttp_statusof 503, and a REST pair on the provider with a REST endpoint are accepted.tests/test_simulator_per_method_overrides.py: a canned 503 whose body holds no error readserror, and a canned 299 whose body holds an error readssuccess.tests/test_simulator_per_method_overrides.py, the heading inprovider_simulator/listeners/jsonrpc.py, the comment above the status check incontrol_api.py, the Tendermint module docstring, anddocs/using_the_simulator.md: the canned reply on each interface, the rule for both status keys, the label of every Tendermint reply, the gRPC refusal, and the cannedbodyin the GraphQL table.How to verify
Results
http_statustests failed before their change: the row readsuccess, and the control API answered HTTP 200 to a gRPC body and to each badhttp_status.errorkey in the body fails the 503 case, a label that calls 299, or every status but 200, an error fails the 299 case, and a Tendermint label that calls only 400 and up an error fails the 302 case.Not in this pull request
http_statusin place ofstatuson JSON-RPC or GraphQL, astatuswith nobody, and a window such asfail_first_nbeside a canned reply, which the canned branch never reads.http_statusoutside 2xx with aresultbody readssuccessthere, anderroron Tendermint RPC, REST and GraphQL.error.