-
Notifications
You must be signed in to change notification settings - Fork 2
Feat(http)/concurrency #22
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This PR introduces comprehensive asynchronous HTTP client support to the Polymarket PHP SDK, enabling efficient concurrent request handling and batching capabilities. The implementation adds new async infrastructure, updates core clients to support async operations, and provides extensive test coverage.
Changes:
- Added async HTTP infrastructure with
AsyncClient,AsyncClientInterface,RequestPool, andBatchResultclasses for managing concurrent requests and batch operations - Updated
GammaandClobclients and their resource classes to accept and use async clients for concurrent operations - Added configuration options (
defaultConcurrency,asyncTimeout) to control async behavior - Updated test infrastructure (
FakeGuzzleHttpClient) to support async methods and added comprehensive test coverage - Fixed endpoint paths in Clob Orders resource from
/ordersto/data/ordersand/data/orderfor better API consistency
Reviewed changes
Copilot reviewed 28 out of 28 changed files in this pull request and generated 9 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Http/AsyncClient.php | New async HTTP client implementation with promise-based request handling and exception mapping |
| src/Http/AsyncClientInterface.php | Interface defining async HTTP operations (getAsync, postAsync, deleteAsync, pool) |
| src/Http/RequestPool.php | Manages concurrent promise execution with configurable concurrency limits |
| src/Http/BatchResult.php | Immutable result container for batch operations, implementing ArrayAccess, Countable, and IteratorAggregate |
| src/Http/FakeGuzzleHttpClient.php | Extended to implement AsyncClientInterface for testing async operations |
| src/Http/GuzzleHttpClient.php | Added getGuzzleClient() method to expose internal Guzzle client for async operations |
| src/Http/Response.php | Enhanced error message to include response body preview for debugging |
| src/Resources/Resource.php | Updated base class to accept optional AsyncClientInterface parameter |
| src/Resources/Gamma/Markets.php | Added async methods (listAsync, getAsync, getBySlugAsync, getMany, getManyBySlug) |
| src/Resources/Clob/Orders.php | Added async methods and fixed endpoint paths to use /data/orders and /data/order |
| src/Resources/Clob/Markets.php | Added async methods (listAsync, getAsync, getMany) |
| src/Gamma.php | Updated to accept and pass AsyncClientInterface to resource classes |
| src/Clob.php | Updated to accept and pass AsyncClientInterface to resource classes |
| src/Client.php | Updated to instantiate and inject async clients for Gamma and Clob |
| src/Config.php | Added defaultConcurrency and asyncTimeout configuration options |
| composer.json | Added guzzlehttp/promises dependency |
| README.md | Updated PHPStan badge from level 9 to level 10 |
| tests/Unit/Http/* | Comprehensive unit tests for AsyncClient, RequestPool, BatchResult, and related classes |
| tests/Feature/Gamma/MarketsAsyncTest.php | Feature tests for async market operations |
| tests/Feature/Clob/OrdersAsyncTest.php | Feature tests for async order operations |
| tests/Feature/Clob/MarketsAsyncTest.php | Feature tests for async Clob market operations |
| tests/Feature/Clob/OrdersTest.php | Updated endpoint paths to match new API structure |
| tests/Feature/ErrorHandlingTest.php | Updated endpoint paths to match new API structure |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| private function mapException(\Throwable $e): PolymarketException | ||
| { | ||
| if (!$e instanceof RequestException) { | ||
| return new PolymarketException($e->getMessage(), (int) $e->getCode()); | ||
| } | ||
|
|
||
| $code = $e->getCode(); | ||
| $message = $e->getMessage(); | ||
|
|
||
| return match (true) { | ||
| $code === 401 || $code === 403 => new AuthenticationException($message, $code), | ||
| $code === 404 => new NotFoundException($message, $code), | ||
| $code === 422 || $code === 400 => new ValidationException($message, $code), | ||
| $code === 429 => new RateLimitException($message, $code), | ||
| $code >= 500 => new ApiException('Server error: ' . $message, $code), | ||
| default => new PolymarketException($message, $code), | ||
| }; |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The HTTP status code is being retrieved using getCode() on the exception, but for Guzzle's RequestException, this may not return the HTTP status code. The status code should be retrieved from the response object if it exists: $e->getResponse()?->getStatusCode(). The current implementation may not correctly map HTTP error codes to the appropriate exception types. Note: This same issue exists in GuzzleHttpClient::handleException() at line 195.
| public function listAsync(array $filters = [], int $limit = 100, int $offset = 0): PromiseInterface | ||
| { | ||
| $params = array_merge($filters, [ | ||
| 'limit' => $limit, | ||
| 'offset' => $offset, | ||
| ]); | ||
|
|
||
| return $this->getAsyncClient()->getAsync('/data/orders', $params) | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| public function getAsync(string $orderId): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync("/data/order/{$orderId}") | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string, mixed> $params | ||
| */ | ||
| public function getOpenAsync(array $params = []): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync('/open-orders', $params) | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string> $orderIds | ||
| */ | ||
| public function getMany(array $orderIds, int $concurrency = 10): BatchResult | ||
| { | ||
| $promises = []; | ||
| foreach ($orderIds as $id) { | ||
| $promises[$id] = $this->getAsync($id); | ||
| } | ||
|
|
||
| return $this->getAsyncClient()->pool($promises, $concurrency); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string> $orderIds | ||
| */ | ||
| public function cancelMany(array $orderIds, int $concurrency = 5): BatchResult | ||
| { | ||
| $promises = []; | ||
| foreach ($orderIds as $id) { | ||
| $promises[$id] = $this->cancelAsync($id); | ||
| } | ||
|
|
||
| return $this->getAsyncClient()->pool($promises, $concurrency); | ||
| } | ||
|
|
||
| private function cancelAsync(string $orderId): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->deleteAsync("/orders/{$orderId}") | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The async methods (listAsync, getAsync, getOpenAsync, getMany, cancelMany) lack PHPDoc comments describing their purpose, return values, and potential exceptions. For consistency with the synchronous methods in this class and better API documentation, add PHPDoc blocks.
| public function listAsync(array $params = []): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync('/markets', $params) | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| public function getAsync(string $conditionId): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync("/market/{$conditionId}") | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string> $conditionIds | ||
| */ | ||
| public function getMany(array $conditionIds, int $concurrency = 10): BatchResult | ||
| { | ||
| $promises = []; | ||
| foreach ($conditionIds as $id) { | ||
| $promises[$id] = $this->getAsync($id); | ||
| } | ||
|
|
||
| return $this->getAsyncClient()->pool($promises, $concurrency); | ||
| } |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The async methods (listAsync, getAsync, getMany) lack PHPDoc comments describing their purpose, return values, and potential exceptions. For consistency with the synchronous methods in this class and better API documentation, add PHPDoc blocks.
| public function listAsync(array $filters = [], int $limit = 100, int $offset = 0): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync('/markets', [ | ||
| 'limit' => $limit, | ||
| 'offset' => $offset, | ||
| ...$filters, | ||
| ])->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| public function getAsync(string $marketId): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync("/markets/$marketId") | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| public function getBySlugAsync(string $slug): PromiseInterface | ||
| { | ||
| return $this->getAsyncClient()->getAsync("/markets/slug/$slug") | ||
| ->then(fn (Response $response): array => $response->json()); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string> $marketIds | ||
| */ | ||
| public function getMany(array $marketIds, int $concurrency = 10): BatchResult | ||
| { | ||
| $promises = []; | ||
| foreach ($marketIds as $id) { | ||
| $promises[$id] = $this->getAsync($id); | ||
| } | ||
|
|
||
| return $this->getAsyncClient()->pool($promises, $concurrency); | ||
| } | ||
|
|
||
| /** | ||
| * @param array<string> $slugs | ||
| */ | ||
| public function getManyBySlug(array $slugs, int $concurrency = 10): BatchResult | ||
| { | ||
| $promises = []; | ||
| foreach ($slugs as $slug) { | ||
| $promises[$slug] = $this->getBySlugAsync($slug); | ||
| } | ||
|
|
||
| return $this->getAsyncClient()->pool($promises, $concurrency); | ||
| } |
Copilot
AI
Jan 26, 2026
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The async methods (listAsync, getAsync, getBySlugAsync, getMany, getManyBySlug) lack PHPDoc comments describing their purpose, return values, and potential exceptions. For consistency with the synchronous methods in this class and better API documentation, add PHPDoc blocks. For example, getAsync should document that it returns a promise that resolves to an array of market data and may throw PolymarketException.
This pull request introduces asynchronous HTTP client support to the Polymarket PHP SDK, enabling efficient batching and concurrent requests. The changes add new interfaces and implementations for async operations, update the core clients (
Gamma,Clob) to accept and use async clients, and provide configuration options for concurrency and timeouts. Additionally, the fake HTTP client used for testing now supports async methods, and the composer dependencies are updated accordingly.Async HTTP support and batching:
AsyncClient(src/Http/AsyncClient.php) andAsyncClientInterface(src/Http/AsyncClientInterface.php) for performing asynchronous HTTP requests and batching via promises. Also introducedBatchResultfor handling batch outcomes. [1] [2] [3]FakeGuzzleHttpClientto implementAsyncClientInterface, supporting async methods and batch processing for testing. [1] [2]Integration with core clients:
GammaandClobclasses to accept and useAsyncClientInterface, passing async clients to resource classes for concurrent operations. [1] [2] [3] [4] [5] [6]Clientclass to instantiate and inject async clients forGammaandClob, including logic for fake/test clients. [1] [2] [3] [4]Configuration and dependency updates:
defaultConcurrency,asyncTimeout) toConfigfor controlling async behavior. [1] [2]guzzlehttp/promisesdependency tocomposer.jsonto support async features.Miscellaneous:
README.mdto reflect level 10 analysis.