Skip to content

fix: Make DynamicEndpoint safe for concurrent requests (#639) - #640

Merged
dragonmantank merged 1 commit into
Vonage:mainfrom
vincent-docto:fix/639-dynamic-endpoint-thread-safety
Oct 14, 2026
Merged

dragonmantank merged 1 commit into
Vonage:mainfrom
vincent-docto:fix/639-dynamic-endpoint-thread-safety

Conversation

@vincent-docto

Copy link
Copy Markdown
Contributor

Fixes #639

Problem

Every endpoint of a client (e.g. MessagesClient.sendMessage, VoiceClient.createCall) is a single
DynamicEndpoint instance shared by all callers. It kept per-request state in two instance fields:

  • cachedRequestBodyString: written then read back inside makeRequest. 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 by parseResponse to update the request object from the response. Under
    concurrency, a response can be written into another caller's request object.

parseResponse also reset both fields in a finally, so one thread could wipe another's state, and
a 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: new protected parseResponse(HttpResponse, REQ), called by execute. It delegates
    to the existing parseResponse(HttpResponse) by default, so existing subclasses and the public API
    are unchanged.
  • DynamicEndpoint: overrides the new method. The single-argument parseResponse(HttpResponse) is kept
    and delegates with a null request. rawRequest on VonageApiResponseException is now rebuilt from
    the 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.
  • Full suite passes (1468 tests).

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

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>
@codecov-commenter

codecov-commenter commented Oct 14, 2026 •

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.44444% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.07%. Comparing base (6564a9e) to head (0d5b561).

Files with missing lines Patch % Lines
...c/main/java/com/vonage/client/DynamicEndpoint.java 93.75% 1 Missing ⚠️
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.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@dragonmantank
dragonmantank merged commit 0402c50 into Vonage:main Oct 14, 2026
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

3 participants