Repository navigation
fix: Make DynamicEndpoint safe for concurrent requests (#639) - #640
Merged
dragonmantank merged 1 commit intoOct 14, 2026
Merged
dragonmantank merged 1 commit into
dragonmantank merged 1 commit into
Conversation
Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
1 of 2 tasks
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #640 +/- ##
============================================
- Coverage 99.07% 99.07% -0.01%
- Complexity 3660 3661 +1
============================================
Files 549 549
Lines 8076 8073 -3
Branches 446 446
============================================
- Hits 8001 7998 -3
Misses 55 55
Partials 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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.
Fixes #639
Problem
Every endpoint of a client (e.g.
MessagesClient.sendMessage,VoiceClient.createCall) is a singleDynamicEndpointinstance shared by all callers. It kept per-request state in two instance fields:cachedRequestBodyString: written then read back insidemakeRequest. Under concurrent calls,one thread can send the body another thread just wrote. The API still answers 2xx, so the failure is silent.
cachedRequestBody: used byparseResponseto update the request object from the response. Underconcurrency, a response can be written into another caller's request object.
parseResponsealso reset both fields in afinally, so one thread could wipe another's state, anda failed HTTP call never reset them at all.
Fix
Remove both fields. The request is now passed to response parsing instead of being stored:
AbstractMethod: newprotected parseResponse(HttpResponse, REQ), called byexecute. It delegatesto the existing
parseResponse(HttpResponse)by default, so existing subclasses and the public APIare unchanged.
DynamicEndpoint: overrides the new method. The single-argumentparseResponse(HttpResponse)is keptand delegates with a
nullrequest.rawRequestonVonageApiResponseExceptionis now rebuilt fromthe request on failure, so it no longer needs the cached string.
No behaviour change for single-threaded callers.
Tests
testConcurrentRequestsKeepTheirOwnBody: 8 threads x 500 iterations through one shared endpoint,checking each HTTP entity matches its own request. Fails on
main(23 mixed bodies in my run), passes with this change.testResponseIsParsedIntoItsOwnRequest: a response is parsed into the request it belongs to.Notes
This does not change the README FAQ guidance on thread safety. Other state in the SDK may still not be
thread-safe. I only audited
DynamicEndpoint.Contribution Checklist
🤖 Generated with Claude Code