Only throw ContextException from receive, send, and join (#166)

This commit is contained in:
Aaron Piotrowski 2023-03-01 23:40:36 -06:00 committed by GitHub
parent 5d975b640b
commit 815c2500bc
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
6 changed files with 49 additions and 53 deletions

View File

@ -16,8 +16,7 @@ interface Context extends Channel
/** /**
* @return TResult The data returned from the context. * @return TResult The data returned from the context.
* *
* @throws ContextException If the context dies unexpectedly. * @throws ContextException If the context exited with an uncaught exception or non-zero code.
* @throws ContextPanicError If the context throws an uncaught exception.
*/ */
public function join(?Cancellation $cancellation = null): mixed; public function join(?Cancellation $cancellation = null): mixed;
} }

View File

@ -36,34 +36,33 @@ abstract class AbstractContext implements Context
} catch (ChannelException $exception) { } catch (ChannelException $exception) {
try { try {
$data = $this->join(new TimeoutCancellation(0.1)); $data = $this->join(new TimeoutCancellation(0.1));
} catch (ContextException|ChannelException|CancelledException) { } catch (ChannelException|CancelledException) {
if (!$this->isClosed()) { if (!$this->isClosed()) {
$this->close(); $this->close();
} }
throw new ContextException( throw new ContextException(
"The process stopped responding, potentially due to a fatal error or calling exit", "The context stopped responding, potentially due to a fatal error or calling exit",
0, previous: $exception,
$exception,
); );
} }
throw new \Error(\sprintf( throw new ContextException(\sprintf(
'Process unexpectedly exited when waiting to receive data with result: %s', 'Context unexpectedly exited when waiting to receive data with result: %s',
flattenArgument($data), flattenArgument($data),
), 0, $exception); ), previous: $exception);
} }
if (!$data instanceof ContextMessage) { if (!$data instanceof ContextMessage) {
if ($data instanceof ExitResult) { if ($data instanceof ExitResult) {
$data = $data->getResult(); $data = $data->getResult();
throw new \Error(\sprintf( throw new ContextException(\sprintf(
'Process unexpectedly exited when waiting to receive data with result: %s', 'Context unexpectedly exited when waiting to receive data with result: %s',
flattenArgument($data), flattenArgument($data),
)); ));
} }
throw new \Error(\sprintf( throw new ContextException(\sprintf(
'Unexpected data type from context: %s', 'Unexpected data type from context: %s',
flattenArgument($data), flattenArgument($data),
)); ));
@ -79,19 +78,19 @@ abstract class AbstractContext implements Context
} catch (ChannelException $exception) { } catch (ChannelException $exception) {
try { try {
$data = $this->join(new TimeoutCancellation(0.1)); $data = $this->join(new TimeoutCancellation(0.1));
} catch (ContextException|ChannelException|CancelledException) { } catch (ChannelException|CancelledException) {
if (!$this->isClosed()) { if (!$this->isClosed()) {
$this->close(); $this->close();
} }
throw new ContextException( throw new ContextException(
"The process stopped responding, potentially due to a fatal error or calling exit", "The context stopped responding, potentially due to a fatal error or calling exit",
0, previous: $exception,
$exception,
); );
} }
throw new \Error(\sprintf( throw new ContextException(\sprintf(
'Process unexpectedly exited when sending data with result: %s', 'Context unexpectedly exited when sending data with result: %s',
flattenArgument($data), flattenArgument($data),
), 0, $exception); ), 0, $exception);
} }
@ -115,7 +114,7 @@ abstract class AbstractContext implements Context
protected function receiveExitResult(?Cancellation $cancellation = null): ExitResult protected function receiveExitResult(?Cancellation $cancellation = null): ExitResult
{ {
if ($this->channel->isClosed()) { if ($this->channel->isClosed()) {
throw new ContextException("The context has already closed"); throw new ContextException("The context has already closed without providing a result");
} }
try { try {
@ -126,7 +125,7 @@ abstract class AbstractContext implements Context
if (!$this->isClosed()) { if (!$this->isClosed()) {
$this->close(); $this->close();
} }
throw new ContextException("Failed to receive result from context", 0, $exception); throw new ContextException("Failed to receive result from context", previous: $exception);
} }
if (!$data instanceof ExitResult) { if (!$data instanceof ExitResult) {
@ -135,13 +134,13 @@ abstract class AbstractContext implements Context
} }
if ($data instanceof ContextMessage) { if ($data instanceof ContextMessage) {
throw new \Error(\sprintf( $data = $data->getMessage();
"The context sent data instead of exiting: %s",
flattenArgument($data),
));
} }
throw new \Error("Did not receive an exit result from context"); throw new ContextException(\sprintf(
"The context sent data instead of exiting: %s",
flattenArgument($data),
));
} }
$this->channel->close(); $this->channel->close();

View File

@ -2,6 +2,7 @@
namespace Amp\Parallel\Context\Internal; namespace Amp\Parallel\Context\Internal;
use Amp\Parallel\Context\ContextException;
use Amp\Parallel\Context\ContextPanicError; use Amp\Parallel\Context\ContextPanicError;
use function Amp\Parallel\Context\flattenThrowableBacktrace; use function Amp\Parallel\Context\flattenThrowableBacktrace;
@ -42,11 +43,16 @@ final class ExitFailure implements ExitResult
} }
/** /**
* @throws ContextPanicError * @throws ContextException
*/ */
public function getResult(): never public function getResult(): never
{ {
throw $this->createException(); $exception = $this->createException();
throw new ContextException(
'Process exited with an uncaught exception: ' . $exception->getMessage(),
previous: $exception,
);
} }
private function createException(): ContextPanicError private function createException(): ContextPanicError

View File

@ -2,7 +2,7 @@
namespace Amp\Parallel\Context\Internal; namespace Amp\Parallel\Context\Internal;
use Amp\Parallel\Context\ContextPanicError; use Amp\Parallel\Context\ContextException;
/** /**
* @internal * @internal
@ -13,7 +13,7 @@ interface ExitResult
/** /**
* @return TValue Return value of the callable given to the execution context. * @return TValue Return value of the callable given to the execution context.
* *
* @throws ContextPanicError If the context exited with an uncaught exception. * @throws ContextException If the context exited with an uncaught exception.
*/ */
public function getResult(): mixed; public function getResult(): mixed;
} }

View File

@ -5,7 +5,6 @@ namespace Amp\Parallel\Test\Context;
use Amp\CancelledException; use Amp\CancelledException;
use Amp\Parallel\Context\Context; use Amp\Parallel\Context\Context;
use Amp\Parallel\Context\ContextException; use Amp\Parallel\Context\ContextException;
use Amp\Parallel\Context\ContextPanicError;
use Amp\PHPUnit\AsyncTestCase; use Amp\PHPUnit\AsyncTestCase;
use Amp\TimeoutCancellation; use Amp\TimeoutCancellation;
use function Amp\async; use function Amp\async;
@ -26,7 +25,7 @@ abstract class AbstractContextTest extends AsyncTestCase
public function testFailingProcess(): void public function testFailingProcess(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage('No string provided'); $this->expectExceptionMessage('No string provided');
$context = $this->createContext(__DIR__ . "/Fixtures/test-process.php"); $context = $this->createContext(__DIR__ . "/Fixtures/test-process.php");
@ -35,40 +34,33 @@ abstract class AbstractContextTest extends AsyncTestCase
public function testThrowingProcessOnReceive(): void public function testThrowingProcessOnReceive(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage('Test message'); $this->expectExceptionMessage('Test message');
$context = $this->createContext(__DIR__ . "/Fixtures/throwing-process.php"); $context = $this->createContext(__DIR__ . "/Fixtures/throwing-process.php");
$cancellation = new TimeoutCancellation(0.1); $cancellation = new TimeoutCancellation(0.1);
try { $context->receive($cancellation);
$context->receive($cancellation);
self::fail('Receiving should have failed'); self::fail('Receiving should have failed');
} catch (ContextException) {
$context->join($cancellation);
}
} }
public function testThrowingProcessOnSend(): void public function testThrowingProcessOnSend(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage('Test message');
$context = $this->createContext(__DIR__ . "/Fixtures/throwing-process.php"); $context = $this->createContext(__DIR__ . "/Fixtures/throwing-process.php");
delay(1); delay(1); // Give process time to start.
try { $context->send(1);
$context->send(1);
self::fail('Sending should have failed'); self::fail('Sending should have failed');
} catch (ContextException) {
$context->join(new TimeoutCancellation(1));
}
} }
public function testInvalidScriptPath(): void public function testInvalidScriptPath(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage("No script found at '../test-process.php'"); $this->expectExceptionMessage("No script found at '../test-process.php'");
$context = $this->createContext("../test-process.php"); $context = $this->createContext("../test-process.php");
@ -77,7 +69,7 @@ abstract class AbstractContextTest extends AsyncTestCase
public function testInvalidResult(): void public function testInvalidResult(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage('The given data could not be serialized'); $this->expectExceptionMessage('The given data could not be serialized');
$context = $this->createContext(__DIR__ . "/Fixtures/invalid-result-process.php"); $context = $this->createContext(__DIR__ . "/Fixtures/invalid-result-process.php");
@ -86,7 +78,7 @@ abstract class AbstractContextTest extends AsyncTestCase
public function testNoCallbackReturned(): void public function testNoCallbackReturned(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage('did not return a callable function'); $this->expectExceptionMessage('did not return a callable function');
$context = $this->createContext(__DIR__ . "/Fixtures/no-callback-process.php"); $context = $this->createContext(__DIR__ . "/Fixtures/no-callback-process.php");
@ -95,7 +87,7 @@ abstract class AbstractContextTest extends AsyncTestCase
public function testParseError(): void public function testParseError(): void
{ {
$this->expectException(ContextPanicError::class); $this->expectException(ContextException::class);
$this->expectExceptionMessage('contains a parse error'); $this->expectExceptionMessage('contains a parse error');
$context = $this->createContext(__DIR__ . "/Fixtures/parse-error-process.inc"); $context = $this->createContext(__DIR__ . "/Fixtures/parse-error-process.inc");

View File

@ -2,7 +2,7 @@
namespace Amp\Parallel\Test\Context\Internal; namespace Amp\Parallel\Test\Context\Internal;
use Amp\Parallel\Context\ContextPanicError; use Amp\Parallel\Context\ContextException;
use Amp\Parallel\Context\Internal\ExitFailure; use Amp\Parallel\Context\Internal\ExitFailure;
use Amp\PHPUnit\AsyncTestCase; use Amp\PHPUnit\AsyncTestCase;
@ -15,7 +15,7 @@ class ExitFailureTest extends AsyncTestCase
$result = new ExitFailure($exception); $result = new ExitFailure($exception);
try { try {
$result->getResult(); $result->getResult();
} catch (ContextPanicError $caught) { } catch (ContextException $caught) {
self::assertGreaterThan(0, \stripos($caught->getMessage(), $message)); self::assertGreaterThan(0, \stripos($caught->getMessage(), $message));
return; return;
} }