Skip to content

Commit 60cfc36

Browse files
BilgeBilge
committed
Added support for RecoverableExceptionHandlers returning promises.
1 parent ca37a22 commit 60cfc36

9 files changed

Lines changed: 262 additions & 107 deletions

‎src/Connector/ImportConnector.php‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -91,19 +91,31 @@ private function createExceptionHandler(): \Closure
9191
{
9292
$userHandlerCloned = $resourceHandlerCloned = false;
9393

94-
return function (\Exception $exception) use (&$userHandlerCloned, &$resourceHandlerCloned): void {
94+
return function (\Exception $exception) use (&$userHandlerCloned, &$resourceHandlerCloned): ?Promise {
9595
// Throw exception instead of retrying, if unrecoverable.
9696
if (!$exception instanceof RecoverableException) {
9797
throw $exception;
9898
}
9999

100100
// Call resource's exception handler, if defined.
101101
if ($this->resourceExceptionHandler) {
102-
self::invokeHandler($this->resourceExceptionHandler, $exception, $resourceHandlerCloned);
102+
$results[] = self::invokeHandler($this->resourceExceptionHandler, $exception, $resourceHandlerCloned);
103103
}
104104

105105
// Call user's exception handler.
106-
self::invokeHandler($this->userExceptionHandler, $exception, $userHandlerCloned);
106+
$results[] = self::invokeHandler($this->userExceptionHandler, $exception, $userHandlerCloned);
107+
108+
/*
109+
* Handlers may return a Promise, but all other return values are discarded. Although the underlying
110+
* library supports returning false, Porter only allows exceptions to short-circuit. However,
111+
* Porter does nothing to restrict promises that return false, although it is discouraged.
112+
*/
113+
return ($promises = array_filter(
114+
$results,
115+
static function ($value): bool {
116+
return $value instanceof Promise;
117+
}
118+
)) ? \Amp\Promise\all($promises) : null;
107119
};
108120
}
109121

@@ -113,19 +125,21 @@ private function createExceptionHandler(): \Closure
113125
* @param RecoverableExceptionHandler $handler Fetch exception handler.
114126
* @param RecoverableException $recoverableException Recoverable exception to pass to the handler.
115127
* @param bool $cloned False if handler requires cloning, true if handler has already been cloned.
128+
*
129+
* @return Promise|null
116130
*/
117131
private static function invokeHandler(
118132
RecoverableExceptionHandler &$handler,
119133
RecoverableException $recoverableException,
120134
bool &$cloned
121-
): void {
135+
): ?Promise {
122136
if (!$cloned && !$handler instanceof StatelessRecoverableExceptionHandler) {
123137
$handler = clone $handler;
124138
$handler->initialize();
125139
$cloned = true;
126140
}
127141

128-
$handler($recoverableException);
142+
return $handler($recoverableException);
129143
}
130144

131145
/**

‎src/Connector/Recoverable/ExponentialSleepRecoverableExceptionHandler.php‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33

44
namespace ScriptFUSION\Porter\Connector\Recoverable;
55

6+
use Amp\Promise;
67
use ScriptFUSION\Retry\ExceptionHandler\ExponentialBackoffExceptionHandler;
78

89
/**
@@ -30,8 +31,8 @@ public function initialize(): void
3031
$this->handler = new ExponentialBackoffExceptionHandler($this->initialDelay);
3132
}
3233

33-
public function __invoke(RecoverableException $exception): void
34+
public function __invoke(RecoverableException $exception): ?Promise
3435
{
35-
($this->handler)($exception);
36+
return ($this->handler)($exception);
3637
}
3738
}

‎src/Connector/Recoverable/RecoverableExceptionHandler.php‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
namespace ScriptFUSION\Porter\Connector\Recoverable;
55

6+
use Amp\Promise;
7+
68
/**
79
* Provides methods for handling recoverable exceptions.
810
*
@@ -26,9 +28,11 @@ interface RecoverableExceptionHandler
2628
public function initialize(): void;
2729

2830
/**
29-
* Handles a recoverable exception.
31+
* Handles a recoverable exception. Return value is ignored unless it's a promise.
32+
*
33+
* @param RecoverableException $exception Recoverable exception.
3034
*
31-
* @return mixed
35+
* @return Promise|null Promise that responds with an asynchronous delay or null.
3236
*/
33-
public function __invoke(RecoverableException $exception);
37+
public function __invoke(RecoverableException $exception): ?Promise;
3438
}

‎src/Connector/Recoverable/StatelessRecoverableExceptionHandler.php‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
namespace ScriptFUSION\Porter\Connector\Recoverable;
55

6+
use Amp\Promise;
7+
68
/**
79
* Contains a fetch exception handler that does not have private state and therefore does not require initialization.
810
*/
@@ -20,8 +22,8 @@ final public function initialize(): void
2022
// Intentionally empty.
2123
}
2224

23-
final public function __invoke(RecoverableException $exception): void
25+
final public function __invoke(RecoverableException $exception): ?Promise
2426
{
25-
($this->handler)($exception);
27+
return ($this->handler)($exception);
2628
}
2729
}

‎test/FixtureFactory.php‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -32,9 +32,7 @@ public static function buildImportConnector(
3232
return new ImportConnector(
3333
$connector,
3434
$context ?: self::buildConnectionContext(),
35-
$recoverableExceptionHandler ?: new StatelessRecoverableExceptionHandler(static function (): void {
36-
// Intentionally empty.
37-
}),
35+
$recoverableExceptionHandler ?: \Mockery::spy(RecoverableExceptionHandler::class),
3836
$maxFetchAttempts
3937
);
4038
}

‎test/Integration/Porter/Connector/ImportConnectorTest.php‎

Lines changed: 207 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,207 @@
1+
<?php
2+
declare(strict_types=1);
3+
4+
namespace ScriptFUSIONTest\Integration\Porter\Connector;
5+
6+
use Mockery\Adapter\Phpunit\MockeryPHPUnitIntegration;
7+
use PHPUnit\Framework\TestCase;
8+
use ScriptFUSION\Porter\Connector\Connector;
9+
use ScriptFUSION\Porter\Connector\ImportConnector;
10+
use ScriptFUSION\Porter\Connector\Recoverable\RecoverableExceptionHandler;
11+
use ScriptFUSION\Porter\Connector\Recoverable\StatelessRecoverableExceptionHandler;
12+
use ScriptFUSION\Retry\FailingTooHardException;
13+
use ScriptFUSIONTest\FixtureFactory;
14+
use ScriptFUSIONTest\MockFactory;
15+
use ScriptFUSIONTest\Stubs\TestRecoverableException;
16+
use ScriptFUSIONTest\Stubs\TestRecoverableExceptionHandler;
17+
18+
/**
19+
* @see ImportConnector
20+
*/
21+
final class ImportConnectorTest extends TestCase
22+
{
23+
use MockeryPHPUnitIntegration;
24+
25+
26+
/**
27+
* Tests that when retry() is called multiple times, the original fetch exception handler is unmodified.
28+
* This is expected because the handler must be cloned using the prototype pattern to ensure multiple concurrent
29+
* fetches do not conflict.
30+
*
31+
* @dataProvider provideHandlerAndContext
32+
*/
33+
public function testFetchExceptionHandlerCloned(
34+
TestRecoverableExceptionHandler $handler,
35+
ImportConnector $connector
36+
): void {
37+
$handler->initialize();
38+
$initial = $handler->getCurrent();
39+
40+
$connector->fetch('foo');
41+
42+
self::assertSame($initial, $handler->getCurrent());
43+
}
44+
45+
public function provideHandlerAndContext(): \Generator
46+
{
47+
yield 'User exception handler' => [
48+
$handler = new TestRecoverableExceptionHandler,
49+
$connector = FixtureFactory::buildImportConnector(
50+
\Mockery::mock(Connector::class)
51+
->shouldReceive('fetch')
52+
->andReturnUsing($this->createExceptionThrowingClosure())
53+
->getMock(),
54+
null,
55+
$handler
56+
),
57+
];
58+
59+
// It should be OK to reuse the handler here because the whole point of the test is that it's not modified.
60+
$connector->setRecoverableExceptionHandler($handler);
61+
yield 'Resource exception handler' => [$handler, $connector];
62+
}
63+
64+
/**
65+
* Tests that when retry() is called, a stateless fetch exception handler is neither cloned nor reinitialized.
66+
* For stateless handlers, initialization is a NOOP, so avoiding cloning is a small optimization.
67+
*/
68+
public function testStatelessExceptionHandlerNotCloned(): void
69+
{
70+
$connector = FixtureFactory::buildImportConnector(
71+
\Mockery::mock(Connector::class)
72+
->shouldReceive('fetch')
73+
->twice()
74+
->andReturnUsing($this->createExceptionThrowingClosure())
75+
->getMock(),
76+
null,
77+
$handler = new StatelessRecoverableExceptionHandler(static function (): void {
78+
// Intentionally empty.
79+
})
80+
);
81+
82+
$connector->fetch('foo');
83+
84+
self::assertSame(
85+
$handler,
86+
\Closure::bind(
87+
function (): RecoverableExceptionHandler {
88+
return $this->userExceptionHandler;
89+
},
90+
$connector,
91+
$connector
92+
)()
93+
);
94+
}
95+
96+
/**
97+
* Tests that a recoverable exception handler cannot return false.
98+
*/
99+
public function testExceptionHandlerCannotCancelRetries(): void
100+
{
101+
$this->expectException(\TypeError::class);
102+
103+
FixtureFactory::buildImportConnector(
104+
\Mockery::mock(Connector::class)
105+
->shouldReceive('fetch')
106+
->andThrow(new TestRecoverableException)
107+
->getMock(),
108+
null,
109+
new StatelessRecoverableExceptionHandler(static function () {
110+
return false;
111+
})
112+
)->fetch('foo');
113+
}
114+
115+
/**
116+
* Tests that when a user recoverable exception handler returns a promise, the promise is resolved.
117+
*/
118+
public function testAsyncUserRecoverableExceptionHandler(): void
119+
{
120+
$connector = FixtureFactory::buildImportConnector(
121+
\Mockery::mock(Connector::class)
122+
->shouldReceive('fetchAsync')
123+
->andThrow(new TestRecoverableException)
124+
->getMock(),
125+
null,
126+
self::createAsyncRecoverableExceptionHandler()
127+
);
128+
129+
try {
130+
\Amp\Promise\wait($connector->fetchAsync('foo'));
131+
} catch (FailingTooHardException $exception) {
132+
// This is fine.
133+
}
134+
135+
self::assertTrue(isset($exception));
136+
}
137+
138+
/**
139+
* Tests that when a resource recoverable exception handler returns a promise, the promise is resolved.
140+
*/
141+
public function testAsyncResourceRecoverableExceptionHandler(): void
142+
{
143+
$connector = FixtureFactory::buildImportConnector(
144+
\Mockery::mock(Connector::class)
145+
->shouldReceive('fetchAsync')
146+
->andThrow(new TestRecoverableException)
147+
->getMock()
148+
);
149+
150+
$connector->setRecoverableExceptionHandler(self::createAsyncRecoverableExceptionHandler());
151+
152+
try {
153+
\Amp\Promise\wait($connector->fetchAsync('foo'));
154+
} catch (FailingTooHardException $exception) {
155+
// This is fine.
156+
}
157+
158+
self::assertTrue(isset($exception));
159+
}
160+
161+
/**
162+
* Tests that when user and resource recoverable exception handlers both return promises, both promises are
163+
* resolved.
164+
*/
165+
public function testAsyncUserAndResourceRecoverablExceptionHandlers(): void
166+
{
167+
$connector = FixtureFactory::buildImportConnector(
168+
\Mockery::mock(Connector::class)
169+
->shouldReceive('fetchAsync')
170+
->andThrow(new TestRecoverableException)
171+
->getMock(),
172+
null,
173+
self::createAsyncRecoverableExceptionHandler()
174+
);
175+
176+
$connector->setRecoverableExceptionHandler(self::createAsyncRecoverableExceptionHandler());
177+
178+
try {
179+
\Amp\Promise\wait($connector->fetchAsync('foo'));
180+
} catch (FailingTooHardException $exception) {
181+
// This is fine.
182+
}
183+
184+
self::assertTrue(isset($exception));
185+
}
186+
187+
/**
188+
* Creates a closure that only throws an exception on the first invocation.
189+
*/
190+
private static function createExceptionThrowingClosure(): \Closure
191+
{
192+
return static function (): void {
193+
static $invocationCount;
194+
195+
if (!$invocationCount++) {
196+
throw new TestRecoverableException;
197+
}
198+
};
199+
}
200+
201+
private static function createAsyncRecoverableExceptionHandler(): RecoverableExceptionHandler
202+
{
203+
return new StatelessRecoverableExceptionHandler(static function () {
204+
return MockFactory::mockPromise();
205+
});
206+
}
207+
}

‎test/MockFactory.php‎

Lines changed: 16 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@
66
use Amp\Delayed;
77
use Amp\Iterator;
88
use Amp\Producer;
9+
use Amp\Promise;
910
use Mockery\MockInterface;
1011
use ScriptFUSION\Porter\Connector\AsyncConnector;
1112
use ScriptFUSION\Porter\Connector\Connector;
@@ -25,7 +26,7 @@ final class MockFactory
2526
*/
2627
public static function mockProvider()
2728
{
28-
return \Mockery::namedMock(uniqid(Provider::class, false), Provider::class, AsyncProvider::class)
29+
return \Mockery::namedMock(uniqid(Provider::class), Provider::class, AsyncProvider::class)
2930
->shouldReceive('getConnector')
3031
->andReturn(
3132
\Mockery::mock(Connector::class)
@@ -75,4 +76,18 @@ public static function mockResource(Provider $provider, \Iterator $return = null
7576

7677
return $resource;
7778
}
79+
80+
/**
81+
* @return Promise|MockInterface
82+
*/
83+
public static function mockPromise()
84+
{
85+
return \Mockery::mock(Promise::class)
86+
->expects('onResolve')
87+
->andReturnUsing(static function (\Closure $onResolve): void {
88+
$onResolve(null, null);
89+
})
90+
->getMock()
91+
;
92+
}
7893
}

0 commit comments

Comments
 (0)