Skip to content

Commit c0ba5c3

Browse files
jsorjsor
committed
Rework done() implementation
done() now only wraps the UnhandledRejectionException if the rejection value isn't an exception. Also, more tests.
1 parent b6d9ef9 commit c0ba5c3

12 files changed

Lines changed: 327 additions & 87 deletions

‎README.md‎

Lines changed: 5 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -607,7 +607,7 @@ $deferred->progress(1); // Prints "Progress 2"
607607

608608
The golden rule is:
609609

610-
Either return your promise, or call `done()` on it.
610+
Either return your promise, or call done() on it.
611611

612612
At a first glance, `then()` and `done()` seem very similar. However, there are
613613
important distinctions.
@@ -663,7 +663,7 @@ getJsonResult()
663663
// Do something with $jsonObject
664664
},
665665
function (ApiErrorException $exception) {
666-
if ($isDebug) {
666+
if (isDebug()) {
667667
throw $e;
668668
} else {
669669
logException($exception);
@@ -672,19 +672,10 @@ getJsonResult()
672672
);
673673
```
674674

675-
Note that exceptions which escape the `$onFulfilled` or `$onRejected` callbacks
676-
are __always__ from type `React\Promise\UnhandledRejectionException`.
675+
Note that if a rejection value is not an instance of `\Exception`, it will be
676+
wrapped in an exception of the type `React\Promise\UnhandledRejectionException`.
677677

678-
You can get the original reason/exception by calling
679-
`$unhandledRejectionException->getReason()`.
680-
681-
```php
682-
try {
683-
getJsonResult()->done();
684-
} catch (React\Promise\UnhandledRejectionException $e) {
685-
echo $e->getReason()->getMessage();
686-
}
687-
```
678+
You can get the original rejection reason by calling `$exception->getReason()`.
688679

689680
Credits
690681
-------

‎src/FulfilledPromise.php‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -36,10 +36,10 @@ public function done(callable $onFulfilled = null, callable $onRejected = null,
3636
return;
3737
}
3838

39-
$this
40-
->then($onFulfilled)
41-
->then(null, function($reason) {
42-
throw new UnhandledRejectionException($reason);
43-
});
39+
$result = $onFulfilled($this->value);
40+
41+
if ($result instanceof PromiseInterface) {
42+
$result->done();
43+
}
4444
}
4545
}

‎src/Promise.php‎

Lines changed: 11 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -39,15 +39,18 @@ public function then(callable $onFulfilled = null, callable $onRejected = null,
3939

4040
public function done(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null)
4141
{
42-
$promise = $this;
43-
44-
if ($onFulfilled || $onRejected || $onProgress) {
45-
$promise = $this->then($onFulfilled, $onRejected, $onProgress);
42+
if (null !== $this->result) {
43+
return $this->result->done($onFulfilled, $onRejected, $onProgress);
4644
}
4745

48-
$promise->then(null, function($reason) {
49-
throw new UnhandledRejectionException($reason);
50-
});
46+
$this->handlers[] = function (PromiseInterface $promise) use ($onFulfilled, $onRejected) {
47+
$promise
48+
->done($onFulfilled, $onRejected);
49+
};
50+
51+
if ($onProgress) {
52+
$this->progressHandlers[] = $onProgress;
53+
}
5154
}
5255

5356
private function resolver(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null)
@@ -68,7 +71,7 @@ private function resolver(callable $onFulfilled = null, callable $onRejected = n
6871
$this->handlers[] = function (PromiseInterface $promise) use ($onFulfilled, $onRejected, $resolve, $reject, $progressHandler) {
6972
$promise
7073
->then($onFulfilled, $onRejected)
71-
->then($resolve, $reject, $progressHandler);
74+
->done($resolve, $reject, $progressHandler);
7275
};
7376

7477
$this->progressHandlers[] = $progressHandler;

‎src/RejectedPromise.php‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -23,25 +23,25 @@ public function then(callable $onFulfilled = null, callable $onRejected = null,
2323
}
2424

2525
return resolve($onRejected($this->reason));
26-
} catch (UnhandledRejectionException $e) {
27-
throw $e;
2826
} catch (\Exception $exception) {
2927
return new RejectedPromise($exception);
3028
}
3129
}
3230

3331
public function done(callable $onFulfilled = null, callable $onRejected = null, callable $onProgress = null)
3432
{
35-
$handler = function($reason) {
36-
throw new UnhandledRejectionException($reason);
37-
};
38-
3933
if (null === $onRejected) {
40-
return $handler($this->reason);
34+
throw UnhandledRejectionException::resolve($this->reason);
4135
}
4236

43-
$this
44-
->then(null, $onRejected)
45-
->then(null, $handler);
37+
$result = $onRejected($this->reason);
38+
39+
if ($result instanceof self) {
40+
throw UnhandledRejectionException::resolve($result->reason);
41+
}
42+
43+
if ($result instanceof PromiseInterface) {
44+
$result->done();
45+
}
4646
}
4747
}

‎src/UnhandledRejectionException.php‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -6,18 +6,22 @@ class UnhandledRejectionException extends \RuntimeException
66
{
77
private $reason;
88

9+
public static function resolve($reason)
10+
{
11+
if ($reason instanceof \Exception) {
12+
return $reason;
13+
}
14+
15+
return new static($reason);
16+
}
17+
918
public function __construct($reason)
1019
{
1120
$this->reason = $reason;
1221

13-
$message = sprintf(
14-
'Unhandled Rejection: %s',
15-
$reason instanceof \Exception ? $reason->getMessage() : json_encode($reason)
16-
);
17-
18-
$previous = $reason instanceof \Exception ? $reason : null;
22+
$message = sprintf('Unhandled Rejection: %s', json_encode($reason));
1923

20-
parent::__construct($message, 0, $previous);
24+
parent::__construct($message, 0);
2125
}
2226

2327
public function getReason()

‎tests/PromiseTest/ProgressTestTrait.php‎

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -290,4 +290,32 @@ public function progressShouldReturnSilentlyOnProgressWhenAlreadyRejected()
290290

291291
$this->assertNull($adapter->progress());
292292
}
293+
294+
/** @test */
295+
public function doneShouldInvokeProgressHandler()
296+
{
297+
$adapter = $this->getPromiseTestAdapter();
298+
299+
$mock = $this->createCallableMock();
300+
$mock
301+
->expects($this->once())
302+
->method('__invoke')
303+
->with($this->identicalTo(1));
304+
305+
$this->assertNull($adapter->promise()->done(null, null, $mock));
306+
$adapter->progress(1);
307+
}
308+
309+
/** @test */
310+
public function doneShouldThrowExceptionThrownProgressHandler()
311+
{
312+
$adapter = $this->getPromiseTestAdapter();
313+
314+
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
315+
316+
$this->assertNull($adapter->promise()->done(null, null, function() {
317+
throw new \Exception('UnhandledRejectionException');
318+
}));
319+
$adapter->progress(1);
320+
}
293321
}

‎tests/PromiseTest/PromiseFulfilledTestTrait.php‎

Lines changed: 5 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -177,23 +177,7 @@ public function thenShouldSwitchFromCallbacksToErrbacksWhenCallbackThrows()
177177
}
178178

179179
/** @test */
180-
public function doneShouldReturnNull()
181-
{
182-
$adapter = $this->getPromiseTestAdapter();
183-
184-
$this->assertNull($adapter->promise()->done());
185-
}
186-
187-
/** @test */
188-
public function doneShouldReturnAllowNull()
189-
{
190-
$adapter = $this->getPromiseTestAdapter();
191-
192-
$this->assertNull($adapter->promise()->done(null, null, null));
193-
}
194-
195-
/** @test */
196-
public function doneShouldInvokeFulfillmentHandler()
180+
public function doneShouldInvokeFulfillmentHandlerForFulfilledPromise()
197181
{
198182
$adapter = $this->getPromiseTestAdapter();
199183

@@ -208,20 +192,20 @@ public function doneShouldInvokeFulfillmentHandler()
208192
}
209193

210194
/** @test */
211-
public function doneShouldBeFatalWhenFulfillmentHandlerThrows()
195+
public function doneShouldThrowExceptionThrownFulfillmentHandlerForFulfilledPromise()
212196
{
213197
$adapter = $this->getPromiseTestAdapter();
214198

215-
$this->setExpectedException('React\\Promise\\UnhandledRejectionException');
199+
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
216200

217201
$adapter->resolve(1);
218202
$this->assertNull($adapter->promise()->done(function() {
219-
throw new \Exception();
203+
throw new \Exception('UnhandledRejectionException');
220204
}));
221205
}
222206

223207
/** @test */
224-
public function doneShouldBeFatalWhenFulfillmentHandlerRejects()
208+
public function doneShouldThrowUnhandledRejectionExceptionWhenFulfillmentHandlerRejectsForFulfilledPromise()
225209
{
226210
$adapter = $this->getPromiseTestAdapter();
227211

‎tests/PromiseTest/PromisePendingTestTrait.php‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,4 +24,20 @@ public function thenShouldReturnAllowNullForPendingPromise()
2424

2525
$this->assertInstanceOf('React\\Promise\\PromiseInterface', $adapter->promise()->then(null, null, null));
2626
}
27+
28+
/** @test */
29+
public function doneShouldReturnNullForPendingPromise()
30+
{
31+
$adapter = $this->getPromiseTestAdapter();
32+
33+
$this->assertNull($adapter->promise()->done());
34+
}
35+
36+
/** @test */
37+
public function doneShouldReturnAllowNullForPendingPromise()
38+
{
39+
$adapter = $this->getPromiseTestAdapter();
40+
41+
$this->assertNull($adapter->promise()->done(null, null, null));
42+
}
2743
}

‎tests/PromiseTest/PromiseRejectedTestTrait.php‎

Lines changed: 47 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
namespace React\Promise\PromiseTest;
44

55
use React\Promise\Deferred;
6-
use React\Promise\UnhandledRejectionException;
76

87
trait PromiseRejectedTestTrait
98
{
@@ -185,7 +184,7 @@ function ($val) {
185184
}
186185

187186
/** @test */
188-
public function doneShouldInvokeRejectionHandler()
187+
public function doneShouldInvokeRejectionHandlerForRejectedPromise()
189188
{
190189
$adapter = $this->getPromiseTestAdapter();
191190

@@ -200,20 +199,31 @@ public function doneShouldInvokeRejectionHandler()
200199
}
201200

202201
/** @test */
203-
public function doneShouldBeFatalWhenRejectionHandlerThrows()
202+
public function doneShouldThrowExceptionThrownByRejectionHandlerForRejectedPromise()
204203
{
205204
$adapter = $this->getPromiseTestAdapter();
206205

207-
$this->setExpectedException('React\\Promise\\UnhandledRejectionException');
206+
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
208207

209208
$adapter->reject(1);
210209
$this->assertNull($adapter->promise()->done(null, function() {
211-
throw new \Exception();
210+
throw new \Exception('UnhandledRejectionException');
212211
}));
213212
}
214213

215214
/** @test */
216-
public function doneShouldBeFatalWhenRejectionHandlerRejects()
215+
public function doneShouldThrowUnhandledRejectionExceptionWhenRejectedWithNonExceptionForRejectedPromise()
216+
{
217+
$adapter = $this->getPromiseTestAdapter();
218+
219+
$this->setExpectedException('React\\Promise\\UnhandledRejectionException');
220+
221+
$adapter->reject(1);
222+
$this->assertNull($adapter->promise()->done());
223+
}
224+
225+
/** @test */
226+
public function doneShouldThrowUnhandledRejectionExceptionWhenRejectionHandlerRejectsForRejectedPromise()
217227
{
218228
$adapter = $this->getPromiseTestAdapter();
219229

@@ -226,20 +236,35 @@ public function doneShouldBeFatalWhenRejectionHandlerRejects()
226236
}
227237

228238
/** @test */
229-
public function doneShouldBeFatalWhenNoRejectionHandlerProvided()
239+
public function doneShouldThrowRejectionExceptionWhenRejectionHandlerRejectsWithExceptionForRejectedPromise()
230240
{
231241
$adapter = $this->getPromiseTestAdapter();
232242

233-
$this->setExpectedException('React\\Promise\\UnhandledRejectionException');
243+
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
234244

235245
$adapter->reject(1);
246+
$this->assertNull($adapter->promise()->done(null, function() {
247+
return \React\Promise\reject(new \Exception('UnhandledRejectionException'));
248+
}));
249+
}
250+
251+
/** @test */
252+
public function doneShouldThrowExceptionProvidedAsRejectionValueForRejectedPromise()
253+
{
254+
$adapter = $this->getPromiseTestAdapter();
255+
256+
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
257+
258+
$adapter->reject(new \Exception('UnhandledRejectionException'));
236259
$this->assertNull($adapter->promise()->done());
237260
}
238261

239262
/** @test */
240-
public function doneShouldBeFatalInDeepNestingPromiseChains()
263+
public function doneShouldThrowWithDeepNestingPromiseChainsForRejectedPromise()
241264
{
242-
$exception = new \Exception('Fatal');
265+
$this->setExpectedException('\Exception', 'UnhandledRejectionException');
266+
267+
$exception = new \Exception('UnhandledRejectionException');
243268

244269
$d = new Deferred();
245270
$d->resolve();
@@ -248,24 +273,24 @@ public function doneShouldBeFatalInDeepNestingPromiseChains()
248273
$d = new Deferred();
249274
$d->resolve();
250275

251-
$identity = function () {
252-
};
253-
254-
return \React\Promise\resolve($d->promise()->then($identity))->then(
255-
function () use($exception) {
276+
return \React\Promise\resolve($d->promise()->then(function () {}))->then(
277+
function () use ($exception) {
256278
throw $exception;
257279
}
258280
);
259281
})));
260282

261-
$catchedException = null;
283+
$result->done();
284+
}
262285

263-
try {
264-
$result->done();
265-
} catch (UnhandledRejectionException $e) {
266-
$catchedException = $e->getReason();
267-
}
286+
/** @test */
287+
public function doneShouldRecoverWhenRejectionHandlerCatchesExceptionForRejectedPromise()
288+
{
289+
$adapter = $this->getPromiseTestAdapter();
290+
291+
$adapter->reject(new \Exception('UnhandledRejectionException'));
292+
$this->assertNull($adapter->promise()->done(null, function(\Exception $e) {
268293

269-
$this->assertEquals($exception, $catchedException);
294+
}));
270295
}
271296
}

0 commit comments

Comments
 (0)