Jens-G opened a new pull request, #3866:
URL: https://github.com/apache/thrift/pull/3866

   JIRA: [THRIFT-6268](https://issues.apache.org/jira/browse/THRIFT-6268)
   Client: erl
   
   This PR covers the first of the three points in THRIFT-6268: a failed 
request. The ticket also names https support and a limit on the size of a 
reply. Both will follow as separate changes.
   
   `thrift_http_transport:flush/1` matched the result of `httpc:request/4` 
against `{ok, {{_, 200, _}, _, Body}}`. Any other status, such as a 404, a 500 
or a redirect, and any error `httpc` returned, such as a refused connection, 
raised a `badmatch` in the calling process. `thrift_client:write_message/5` 
catches only `{error, _}` results from the transport, so the `badmatch` reached 
the caller of `thrift_client:call/3`.
   
   ### Change
   
   `flush/1` now inspects the result:
   - **A 200 reply** is appended to the read buffer, as before.
   - **Any other status** returns `{error, {http_status, Status, 
ReasonPhrase}}`.
   - **An `httpc` error** returns `{error, Reason}`.
   
   `thrift_client:call/3` hands either back as `{Client, {error, ...}}`. The 
write buffer is cleared in all three cases, so what was written for a failed 
request is not sent again with the next request.
   
   ### Tests
   
   New `lib/erl/test/test_thrift_http_transport.erl`. The module had no tests 
before. The new tests run against a local HTTP server built on `gen_tcp`:
   - A 200 reply is read back, and the server received the request body.
   - 500, 404 and 302 replies return `{error, {http_status, Code, Reason}}`.
   - A port with nothing listening returns `{error, _}`.
   - After a 503, the next request carries only what was written for it.
   - `thrift_client:call(Client, testVoid, [])` against a 500 reply returns 
`{error, {http_status, 500, _}}`.
   
   Results:
   - **Against the unmodified library:** six of the seven cases fail with a 
`badmatch`; only the 200 case passes.
   - **Mutation:** keeping the write buffer after a failed request makes the 
second request carry `firstsecond`.
   - **`rebar3 eunit`:** all 359 tests pass on OTP 25.3 with rebar3 3.18.0, and 
on OTP 26, 27 and 28.
     - Locally the slim OTP 26+ images needed `ca-certificates`: from OTP 26 
on, `httpc` loads the system CA store for every request. The CI runners have it.
   - **erlfmt:** `rebar3 fmt -c` reports nothing for the changed files. It 
still flags the untouched `src/thrift_binary_protocol.erl`, as on master.
   
   🤖 Generated with [Claude Code](https://claude.com/claude-code)
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]

Reply via email to