8000 [HttpClient] yield a last chunk for completed responses also · symfony/symfony@e11ef7e · GitHub
[go: up one dir, main page]
Skip to content

Commit e11ef7e

Browse files
nicolas-grekasnicolas-grekas
committed
[HttpClient] yield a last chunk for completed responses also
1 parent f54c89c commit e11ef7e

File tree

4 files changed

+38
-19
lines changed

4 files changed

+38
-19
lines changed

‎src/Symfony/Component/HttpClient/Response/CurlResponse.php

Lines changed: 11 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -80,11 +80,12 @@ public function __construct(\stdClass $multi, $ch, array $options = null, string
8080
if ($onProgress = $options['on_progress']) {
8181
$url = isset($info['url']) ? ['url' => $info['url']] : [];
8282
curl_setopt($ch, CURLOPT_NOPROGRESS, false);
83-
curl_setopt($ch, CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url) {
83+
curl_setopt($ch, CURLOPT_PROGRESSFUNCTION, static function ($ch, $dlSize, $dlNow) use ($onProgress, &$info, $url, $multi) {
8484
try {
8585
$onProgress($dlNow, $dlSize, $url + curl_getinfo($ch) + $info);
8686
} catch (\Throwable $e) {
87-
$info['error'] = $e;
87+
$multi->handlesActivity[(int) $ch][] = null;
88+
$multi->handlesActivity[(int) $ch][] = $e;
8889

8990
return 1; // Abort the request
9091
}
@@ -109,6 +110,7 @@ public function __construct(\stdClass $multi, $ch, array $options = null, string
109110
}
110111
self::stream([$response])->current();
111112
} catch (\Throwable $e) {
113+
// Persist timeouts thrown during initialization
112114
$response->info['error'] = $e->getMessage();
113115
$response->close();
114116
throw $e;
@@ -201,13 +203,17 @@ protected function close(): void
201203
*/
202204
protected static function schedule(self $response, array &$runningResponses): void
203205
{
204-
if ('' === curl_getinfo($ch = $response->handle, CURLINFO_PRIVATE)) {
205-
// no-op - response already completed
206-
} elseif (isset($runningResponses[$i = (int) $response->multi->handle])) {
206+
if (isset($runningResponses[$i = (int) $response->multi->handle])) {
207207
$runningResponses[$i][1][$response->id] = $response;
208208
} else {
209209
$runningResponses[$i] = [$response->multi, [$response->id => $response]];
210210
}
211+
212+
if ('' === curl_getinfo($ch = $response->handle, CURLINFO_PRIVATE)) {
213+
// Response already completed
214+
$response->multi->handlesActivity[$response->id][] = null;
215+
$response->multi->handlesActivity[$response->id][] = null !== $response->info['error'] ? new TransportExc 8000 eption($response->info['error']) : null;
216+
}
211217
}
212218

213219
/**

‎src/Symfony/Component/HttpClient/Response/NativeResponse.php

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -165,10 +165,6 @@ private function close(): void
165165
*/
166166
private static function schedule(self $response, array &$runningResponses): void
167167
{
168-
if (null === $response->buffer) {
169-
return;
170-
}
171-
172168
if (!isset($runningResponses[$i = $response->multi->id])) {
173169
$runningResponses[$i] = [$response->multi, []];
174170
}
@@ -178,6 +174,12 @@ private static function schedule(self $response, array &$runningResponses): void
178174
} else {
179175
$runningResponses[$i][1][$response->id] = $response;
180176
}
177+
178+
if (null === $response->buffer) {
179+
// Response already completed
180+
$response->multi->handlesActivity[$response->id][] = null;
181+
$response->multi->handlesActivity[$response->id][] = null !== $response->info['error'] ? new TransportException($response->info['error']) : null;
182+
}
181183
}
182184

183185
/**

‎src/Symfony/Component/HttpClient/Response/ResponseTrait.php

Lines changed: 10 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -100,14 +100,16 @@ public function getContent(bool $throw = true): string
100100
}
101101

102102
if (null === $this->content) {
103-
$content = '';
103+
$content = null;
104104
$chunk = null;
105105

106106
foreach (self::stream([$this]) as $chunk) {
107-
$content .= $chunk->getContent();
107+
if (!$chunk->isLast()) {
108+
$content .= $chunk->getContent();
109+
}
108110
}
109111

110-
if (null === $chunk) {
112+
if (null === $content) {
111113
throw new TransportException('Cannot get the content of the response twice: the request was issued with option "buffer" set to false.');
112114
}
113115

@@ -280,22 +282,21 @@ public static function stream(iterable $responses, float $timeout = null): \Gene
280282
$response->offset += \strlen($chunk);
281283
$chunk = new DataChunk($response->offset, $chunk);
282284
} elseif (null === $chunk) {
283-
if (null !== $e = $response->info['error'] ?? $multi->handlesActivity[$j][0]) {
285+
$e = $multi->handlesActivity[$j][0];
286+
unset($responses[$j], $multi->handlesActivity[$j]);
287+
$response->close();
288+
289+
if (null !== $e) {
284290
$response->info['error'] = $e->getMessage();
285291

286292
if ($e instanceof \Error) {
287-
unset($responses[$j], $multi->handlesActivity[$j]);
288-
$response->close();
289293
throw $e;
290294
}
291295

292296
$chunk = new ErrorChunk($didThrow, $response->offset, $e);
293297
} else {
294298
$chunk = new LastChunk($response->offset);
295299
}
296-
297-
unset($responses[$j]);
298-
$response->close();
299300
} elseif ($chunk instanceof ErrorChunk) {
300301
unset($responses[$j]);
301302
$isTimeout = true;

‎src/Symfony/Contracts/HttpClient/Test/HttpClientTestCase.php

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -194,8 +194,8 @@ public function testDnsError()
194194
$this->assertSame($response, $r);
195195
$this->assertNotNull($chunk->getError());
196196

197+
$this->expectException(TransportExceptionInterface::class);
197198
foreach ($client->stream($response) as $chunk) {
198-
$this->fail('Already errored responses shouldn\'t be yielded');
199199
}
200200
}
201201

@@ -340,6 +340,16 @@ public function testStream()
340340

341341
$this->assertSame($response, $r);
342342
$this->assertSame(['f', 'l'], $result);
343+
344+
$chunk = null;
345+
$i = 0;
346+
347+
foreach ($client->stream($response) as $chunk) {
348+
++$i;
349+
}
350+
351+
$this->assertSame(1, $i);
352+
$this->assertTrue($chunk->isLast());
343353
}
344354

345355
public function testAddToStream()

0 commit comments

Comments
 (0)
0