feat(arr): inject async HTTP transport via ArrTransportInterface (F2b/B1/P1)#17
Merged
Conversation
…/B1/P1)
Decouple the four *arr clients from blocking cURL so event-loop consumers can
inject a non-blocking transport. Closes findings B1/P1/CQ1/F2 (lib side).
- New Arr\Transport\ArrTransportInterface: request(method,url,headers,?body)
returns array{status:int, body:string}.
- New default Arr\Transport\CurlArrTransport implementing the prior blocking
cURL behaviour (cURL call moved out of AbstractArrClient). Documented as
CLI/test only.
- AbstractArrClient: appended OPTIONAL ?ArrTransportInterface $transport = null
ctor param (BC-safe); all I/O flows through the transport; null falls back to
CurlArrTransport so direct instantiation is unchanged. When a transport is
injected, no curl_exec is reached.
- ArrClientFactory: accepts + propagates an optional transport to every client.
- composer.json: keep "zero I/O by charter" honest — ext-curl moved to suggest/
require-dev (only the bundled CurlArrTransport needs it); suggest
workerman/http-client for event-loop consumers.
- Tests: deterministic in-memory FakeArrTransport + InjectedTransportTest
asserting the injected transport short-circuits cURL entirely (canned response
against an unroutable URL), POST body encoding, status-error mapping, and a
control test proving the default transport performs real cURL. Existing
tests/Arr/* still pass with the default transport.
No required method added to ArrClientInterface (transport is a ctor concern) —
additive/minor, not breaking.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #17 +/- ##
============================================
+ Coverage 75.38% 79.89% +4.51%
- Complexity 345 354 +9
============================================
Files 39 40 +1
Lines 967 985 +18
============================================
+ Hits 729 787 +58
+ Misses 238 198 -40
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
Not up to standards ⛔🔴 Issues
|
| Category | Results |
|---|---|
| Complexity | 2 medium |
| Comprehensibility | 1 minor |
🟢 Metrics 19 complexity · 0 duplication
Metric Results Complexity 19 Duplication 0
NEW Get contextual insights on your PRs based on Codacy's metrics, along with PR and Jira context, without leaving GitHub. Enable AI reviewer
TIP This summary will be updated as you push new changes.
detain
added a commit
that referenced
this pull request
Jun 28, 2026
Bump Version::VERSION to 0.11.0 and promote the [Unreleased] CHANGELOG section to [0.11.0] - 2026-06-28, rolling up the four changes merged since v0.10.1: - #14 S1: harden RelayHttpRequest with path/method gate + forbidden-header helpers (isForbiddenHeader/withoutForbiddenHeaders/assertSafe) - #15 JwtClaims: strict-aud variant (fromPayloadStrict) + verification/ round-trip docs (S4+B4) - #16 refactor(arr): extract AbstractArrClient to dedup the four *arr clients (F2a) - #17 feat(arr): inject async HTTP transport via ArrTransportInterface (F2b/B1/P1) composer.json carries no hardcoded version field (tags drive version), so it is left unchanged. The git tag is applied by the coordinator after merge. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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.
Step F2b/B1/P1 — inject async transport via
ArrTransportInterfaceFindings: B1 (HIGH), P1 (HIGH), CQ1, F2 from
findings/plan_phlix-shared.md(Phase 2). Depends on F2a (AbstractArrClientextraction, already merged #16).The four
Arr/*clients did a blocking synchronouscurl_exec()(30s timeout) inside a Workerman/Webman resident-memory worker, contradicting the package's "zero I/O" charter and stalling all coroutines on that worker when an *arr instance is slow. This PR moves that blocking cURL behind an injectable transport seam (lib side only).Changes
src/Arr/Transport/ArrTransportInterface.php—request(string $method, string $url, array $headers, ?string $body): array{status:int, body:string}. Implementations return status + raw body (they do NOT throw on non-2xx; the client maps status codes).src/Arr/Transport/CurlArrTransport.php— the prior blocking cURL behaviour, moved out ofAbstractArrClient::request(). Documented as CLI/test only.AbstractArrClient— appended an OPTIONAL?ArrTransportInterface $transport = nullctor param (BC-safe). All requests dispatch through the transport; when null it falls back tonew CurlArrTransport($timeout)so existing direct instantiation is unchanged. Nocurl_execruns when a transport is injected.ArrClientFactory— appended optional?ArrTransportInterface $transport, propagated to every created client.ArrClientInterfaceunchanged — transport is a constructor concern, NOT a new interface method, so this is additive / minor, not breaking.composer.json— reconciled the "zero I/O" framing: description now states the only bundled network code is the blockingCurlArrTransport(CLI/test only) and event-loop consumers MUST inject an async transport.ext-curlmovedrequire→require-dev+suggest(needed only for the default transport); added asuggestforworkerman/http-client.[Unreleased]note (additive/minor; no version bump in this PR).How verified (full gate, all green)
composer test— OK (354 tests, 2854 assertions) — includes newtests/Arr/Transport/InjectedTransportTest.php; existingtests/Arr/*pass unchanged with the default transport.composer stan— PHPStan Level 9, no baseline: No errors.composer cs— phpcs PSR-12 clean (src + verified new test files).composer psalm— No errors found.Success conditions met
FakeArrTransportmakestests/Arr/*paths fully deterministic (closes CQ5).InjectedTransportTest::testInjectedTransportShortCircuitsCurlasserts that injecting a fake transport short-circuits cURL entirely — the client is pointed at an unroutable URL (http://0.0.0.0:1) and still succeeds via the canned response, proving nocurl_execis reached.testDefaultTransportPerformsRealCurlproves the defaultCurlArrTransportDOES perform real cURL (raises a transport-level "cURL error" against the same unroutable URL) — so the fake genuinely replaces cURL.Coupling / consumer follow-up (separate PRs — the real "stop blocking the event loop" fix)
This PR is the lib seam only. The actual fix lands in the consumers:
workerman/http-client-backedWorkermanArrTransportand inject it (viaArrClientFactory/ direct construction inCustomFormatSyncer,Application, etc.) so *arr calls stop stalling the worker.RequestManager/HubServicesProvider*arr usage and inject the same async transport.workerman/http-clientbelongs in the consumer composer files, NOT this dependency-light lib.Per plan: minor bump + re-pin consumers after merge. Do not merge here — the Phase Coordinator owns the git cycle.
🤖 Generated with Claude Code