test(wallet): make TransferPreapproval failure test robust to validator timeout#5988
Draft
samsondav wants to merge 1 commit into
Draft
Conversation
…or timeout The induced-failure path (validator automation paused) is expected to surface as an HTTP 429 once the server-side retry loop gives up. Under CI load the validator's HTTP request timeout can fire first: the server logs a WARN "resulted in a timeout" and returns a 503, so the strict ordered log assertion saw a WARN entry where it expected the ERROR 429 and failed. Use assertThrowsAndLogsUnorderedOptional so the test accepts either failure (429 or the timeout 503, both logged as ERROR) and tolerates the optional server-side timeout WARN, without weakening the requirement that a genuine failure is surfaced and logged. Signed-off-by: Sam Davies <sam@avrofi.com>
Contributor
|
I believe this is already fixed in #5940. have you still encountered this issue after rebasing to include that change? |
Contributor
Author
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.
Make the "Failure to complete TransferPreapproval creation should be handled correctly" wallet test robust to a transient validator HTTP timeout that flakes the log assertion under CI load.
Flake
Observed on the
wall-clock-time (0)job: https://github.com/canton-network/splice/actions/runs/27450544466 (job: https://github.com/canton-network/splice/actions/runs/27450544466/job/81436663390)The test pauses validator automation so
createTransferPreapprovalcannot complete. The intended outcome is an HTTP 429 once the server-sideRetryFor.ClientCallsloop inHttpWalletHandlergives up (ABORTED->TooManyRequests). Under load the validator's HTTP request timeout fired first:HttpErrorHandler.timeoutHandlerlogged a WARN "resulted in a timeout after 38 seconds" and returned 503 instead. The strict ordered assertion inassertThrowsAndLogsCommandFailuresthen matched the unexpected WARN entry at index 0 witherrorMessage, which requires ERROR level, and failed.Fix
Assert the failure via
assertThrowsAndLogsUnorderedOptional[CommandFailure]: a required ERROR entry matching either "429 Too Many Requests" or "503 Service Unavailable", plus an optional WARN entry matching the server-side "resulted in a timeout". This keeps the requirement that a genuine failure is surfaced and logged as ERROR, while tolerating the transient timeout WARN/503 on a slow host. Mirrors the existing pattern inScanIntegrationTestandSplitwellUpgradeIntegrationTest.Observed on PR #5937's CI run. The fix is by inspection; the integration test was not run locally.