Skip to content

fix: return CURLM_OK from intercepted curl_multi_add_handle#11

Merged
sf-cg merged 3 commits into
CoverGenius:masterfrom
grantoz:fix/curl-multi-add-handle-return-value
Jul 16, 2026
Merged

fix: return CURLM_OK from intercepted curl_multi_add_handle#11
sf-cg merged 3 commits into
CoverGenius:masterfrom
grantoz:fix/curl-multi-add-handle-return-value

Conversation

@grantoz

@grantoz grantoz commented Jul 15, 2026

Copy link
Copy Markdown

The real curl_multi_add_handle() returns CURLM_OK (0) on success. The CurlHook interception was declared : void and returned null.

Guzzle >= 7.13 (CurlMultiHandler::addCurlHandle) strictly checks \CURLM_OK !== $result and throws a RequestException otherwise. Under php-vcr this call is rewritten to CurlHook::curl_multi_add_handle, so the null return produced:

Unable to add the cURL handle to the cURL multi handler: No error (0).

Returning \CURLM_OK mirrors the real function's contract. Older Guzzle (<= 7.9.x) discards the return value, so this change should be safely backwards-compatible.

Grant Ozolins added 3 commits July 15, 2026 16:44
The real curl_multi_add_handle() returns CURLM_OK (0) on success. The
CurlHook interception was declared : void and returned null.

Guzzle >= 7.13 (CurlMultiHandler::addCurlHandle) strictly checks
`\CURLM_OK !== $result` and throws a RequestException otherwise. Under
php-vcr this call is rewritten to CurlHook::curl_multi_add_handle, so the
null return produced:

  Unable to add the cURL handle to the cURL multi handler: No error (0).

Returning \CURLM_OK mirrors the real function's contract. Older Guzzle
(<= 7.9.x) discards the return value, so this is backwards-compatible.
@sf-cg
sf-cg self-requested a review July 16, 2026 00:06
@sf-cg sf-cg self-assigned this Jul 16, 2026
@sf-cg
sf-cg merged commit a3c2542 into CoverGenius:master Jul 16, 2026
81 of 90 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

Development

Successfully merging this pull request may close these issues.

2 participants