Skip to content

Bound status requests and retry them until the deadline - #201

Merged
christian-rogobete merged 1 commit into
mainfrom
hardening-poll-deadline
Nov 5, 2026
Merged

christian-rogobete merged 1 commit into
mainfrom
hardening-poll-deadline

Conversation

@christian-rogobete

Copy link
Copy Markdown
Member

Fixes a defect in the status polling of AssembledTransaction. Before this change, pollStatus sent its status requests through SorobanServer::getTransaction without a bound of their own, so a server that accepted the connection and never answered blocked send and signAndSend indefinitely under the default Guzzle client. A status request that threw ended the call with that exception although the transaction could still complete, and an RPC error response came back as the result with a null status. The waits between requests grew from one second by a factor of 1.5, and the timeout message carried a line break.

This change:

  • Polls every three seconds until timeoutInSeconds has elapsed on a monotonic clock, with the first request at once and one last request at the deadline, as js-stellar-sdk does.
  • Adds SorobanServer::getTransactionWithTimeout(), which bounds a status request by the given time and by the client's own timeout and connect_timeout, and polls through it; getTransaction keeps its behavior and shares the request path.
  • Retries a status request that throws a GuzzleException, RuntimeException or InvalidArgumentException or returns an RPC error until the deadline, and names the last error in the timeout exception, on one line.
  • Leaves the send step and its PENDING and DUPLICATE handling unchanged.
  • Adds tests with Guzzle's MockHandler for failing, RPC-error and FAILED responses, for the deadline with the last error and the request bounds, for a timeoutInSeconds of 0, and for the client-limit clamp of getTransactionWithTimeout. The unit suite note in AGENTS.md names the ten seconds of real waits these tests add.

A SorobanServer subclass or test double that customizes the poll's status request overrides getTransactionWithTimeout(), because getTransaction is no longer on the poll path.

The unit suite passes. The integration class SorobanClientTest ran against testnet on 2026-11-05 and passes.

Closes #200

Fixes a defect in the status polling of `AssembledTransaction`. Before
this change, `pollStatus` sent its status requests through
`SorobanServer::getTransaction` without a bound of their own, so a
server that accepted the connection and never answered blocked `send`
and `signAndSend` indefinitely under the default Guzzle client. A status
request that threw ended the call with that exception although the
transaction could still complete, and an RPC error response came back as
the result with a null status. The waits between requests grew from one
second by a factor of 1.5, and the timeout message carried a line break.

This change:

- Polls every three seconds until `timeoutInSeconds` has elapsed on a
  monotonic clock, with the first request at once and one last request
  at the deadline, as js-stellar-sdk does.
- Adds `SorobanServer::getTransactionWithTimeout()`, which bounds a
  status request by the given time and by the client's own `timeout` and
  `connect_timeout`, and polls through it; `getTransaction` keeps its
  behavior and shares the request path.
- Retries a status request that throws a `GuzzleException`,
  `RuntimeException` or `InvalidArgumentException` or returns an RPC
  error until the deadline, and names the last error in the timeout
  exception, on one line.
- Leaves the send step and its PENDING and DUPLICATE handling unchanged.
- Adds tests with Guzzle's `MockHandler` for failing, RPC-error and
  FAILED responses, for the deadline with the last error and the request
  bounds, for a `timeoutInSeconds` of 0, and for the client-limit clamp
  of `getTransactionWithTimeout`. The unit suite note in `AGENTS.md`
  names the ten seconds of real waits these tests add.

A `SorobanServer` subclass or test double that customizes the poll's
status request overrides `getTransactionWithTimeout()`, because
`getTransaction` is no longer on the poll path.

The unit suite passes. The integration class `SorobanClientTest` ran
against testnet on 2026-11-05 and passes.

Closes #200
@codecov

codecov Bot commented Nov 5, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 93.47%. Comparing base (b55201c) to head (64dc076).

Additional details and impacted files

Impacted file tree graph

@@             Coverage Diff              @@
##               main     #201      +/-   ##
============================================
+ Coverage     93.46%   93.47%   +0.01%     
- Complexity    21609    21621      +12     
============================================
  Files          1006     1006              
  Lines         52399    52422      +23     
============================================
+ Hits          48974    49001      +27     
+ Misses         3425     3421       -4     
Files with missing lines Coverage Δ
...ellarSDK/Soroban/Contract/AssembledTransaction.php 83.18% <100.00%> (+1.44%) ⬆️
Soneso/StellarSDK/Soroban/SorobanServer.php 93.51% <100.00%> (+1.21%) ⬆️
🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@christian-rogobete
christian-rogobete merged commit 90b0041 into main Nov 5, 2026
10 checks passed
@christian-rogobete
christian-rogobete deleted the hardening-poll-deadline branch November 5, 2026 02:32
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

signAndSend never returns when a status request stalls and ends on the first failed one

1 participant