Skip to content
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

Expose next retry delay on Application Failure #511

Merged
merged 5 commits into from
Sep 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 0 additions & 6 deletions psalm-baseline.xml
Original file line number Diff line number Diff line change
Expand Up @@ -363,14 +363,8 @@
<PossiblyNullReference>
<code><![CDATA[getName]]></code>
<code><![CDATA[getName]]></code>
<code><![CDATA[getNonRetryable]]></code>
<code><![CDATA[getScheduledEventId]]></code>
<code><![CDATA[getStackTrace]]></code>
<code><![CDATA[getWorkflowExecution]]></code>
<code><![CDATA[getWorkflowId]]></code>
<code><![CDATA[hasDetails]]></code>
<code><![CDATA[hasDetails]]></code>
<code><![CDATA[hasLastHeartbeatDetails]]></code>
<code><![CDATA[hasLastHeartbeatDetails]]></code>
<code><![CDATA[setStackTrace]]></code>
</PossiblyNullReference>
Expand Down
14 changes: 13 additions & 1 deletion src/Exception/Failure/ApplicationFailure.php
Original file line number Diff line number Diff line change
Expand Up @@ -50,15 +50,17 @@ class ApplicationFailure extends TemporalFailure
* @param string $message
* @param string $type
* @param bool $nonRetryable
* @param ValuesInterface|null $details
* @param ValuesInterface|null $details Optional details about the failure.
* @param \Throwable|null $previous
* @param \DateInterval|null $nextRetryDelay Delay before the next retry attempt.
*/
public function __construct(
string $message,
string $type,
bool $nonRetryable,
ValuesInterface $details = null,
\Throwable $previous = null,
private ?\DateInterval $nextRetryDelay = null,
) {
parent::__construct(
self::buildMessage(\compact('message', 'type', 'nonRetryable')),
Expand Down Expand Up @@ -87,6 +89,11 @@ public function getDetails(): ValuesInterface
return $this->details;
}

public function getNextRetryDelay(): ?\DateInterval
{
return $this->nextRetryDelay;
}

/**
* @return bool
*/
Expand All @@ -110,4 +117,9 @@ public function setDataConverter(DataConverterInterface $converter): void
{
$this->details->setDataConverter($converter);
}

public function setNextRetryDelay(?\DateInterval $nextRetryDelay): void
{
$this->nextRetryDelay = $nextRetryDelay;
}
}
18 changes: 14 additions & 4 deletions src/Exception/Failure/FailureConverter.php
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@
use Temporal\DataConverter\DataConverterInterface;
use Temporal\DataConverter\EncodedValues;
use Temporal\Exception\Client\ActivityCanceledException;
use Temporal\Internal\Support\DateInterval;

final class FailureConverter
{
Expand Down Expand Up @@ -86,6 +87,10 @@ public static function mapExceptionToFailure(\Throwable $e, DataConverterInterfa
$info->setType($e->getType());
$info->setNonRetryable($e->isNonRetryable());

// Set Next Retry Delay
$nextRetry = DateInterval::toDuration($e->getNextRetryDelay());
$nextRetry === null or $info->setNextRetryDelay($nextRetry);

if (!$e->getDetails()->isEmpty()) {
$info->setDetails($e->getDetails()->toPayloads());
}
Expand Down Expand Up @@ -192,22 +197,24 @@ private static function createFailureException(Failure $failure, DataConverterIn
switch (true) {
case $failure->hasApplicationFailureInfo():
$info = $failure->getApplicationFailureInfo();
\assert($info instanceof ApplicationFailureInfo);

$details = $info->hasDetails()
? EncodedValues::fromPayloads($info->getDetails(), $converter)
: EncodedValues::empty()
;
: EncodedValues::empty();

return new ApplicationFailure(
$failure->getMessage(),
$info->getType(),
$info->getNonRetryable(),
$details,
$previous,
DateInterval::parseOrNull($info->getNextRetryDelay()),
);

case $failure->hasTimeoutFailureInfo():
$info = $failure->getTimeoutFailureInfo();
\assert($info instanceof TimeoutFailureInfo);

$details = $info->hasLastHeartbeatDetails()
? EncodedValues::fromPayloads($info->getLastHeartbeatDetails(), $converter)
Expand All @@ -218,6 +225,7 @@ private static function createFailureException(Failure $failure, DataConverterIn

case $failure->hasCanceledFailureInfo():
$info = $failure->getCanceledFailureInfo();
\assert($info instanceof CanceledFailureInfo);

$details = $info->hasDetails()
? EncodedValues::fromPayloads($info->getDetails(), $converter)
Expand All @@ -231,14 +239,14 @@ private static function createFailureException(Failure $failure, DataConverterIn

case $failure->hasServerFailureInfo():
$info = $failure->getServerFailureInfo();
\assert($info instanceof ServerFailureInfo);
return new ServerFailure($failure->getMessage(), $info->getNonRetryable(), $previous);

case $failure->hasResetWorkflowFailureInfo():
$info = $failure->getResetWorkflowFailureInfo();
$details = $info->hasLastHeartbeatDetails()
? EncodedValues::fromPayloads($info->getLastHeartbeatDetails(), $converter)
: EncodedValues::empty()
;
: EncodedValues::empty();

return new ApplicationFailure(
$failure->getMessage(),
Expand All @@ -250,6 +258,7 @@ private static function createFailureException(Failure $failure, DataConverterIn

case $failure->hasActivityFailureInfo():
$info = $failure->getActivityFailureInfo();
\assert($info instanceof ActivityFailureInfo);

return new ActivityFailure(
$info->getScheduledEventId(),
Expand All @@ -264,6 +273,7 @@ private static function createFailureException(Failure $failure, DataConverterIn
case $failure->hasChildWorkflowExecutionFailureInfo():
$info = $failure->getChildWorkflowExecutionFailureInfo();
$execution = $info->getWorkflowExecution();
\assert($execution instanceof WorkflowExecution);

return new ChildWorkflowFailure(
$info->getInitiatedEventId(),
Expand Down
3 changes: 2 additions & 1 deletion src/Exception/Failure/TemporalFailure.php
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ public function setOriginalStackTrace(string $stackTrace): void
}

/**
* @return bool
* @psalm-assert-if-true non-empty-string $this->originalStackTrace
* @psalm-assert-if-false null $this->originalStackTrace
*/
public function hasOriginalStackTrace(): bool
{
Expand Down
8 changes: 7 additions & 1 deletion src/Internal/Support/DateInterval.php
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@

/**
* @psalm-type DateIntervalFormat = DateInterval::FORMAT_*
* @psalm-type DateIntervalValue = string | int | float | \DateInterval
* @psalm-type DateIntervalValue = string | int | float | \DateInterval | Duration
*/
final class DateInterval
{
Expand Down Expand Up @@ -100,6 +100,12 @@ public static function parse($interval, string $format = self::FORMAT_MILLISECON
seconds: $seconds % 60,
microseconds: $micros % 1000_000,
);

case $interval instanceof Duration:
return self::parse(
$interval->getSeconds() * 1e6 + $interval->getNanos() / 1e3,
self::FORMAT_MICROSECONDS,
);
default:
throw new \InvalidArgumentException(self::ERROR_INVALID_DATETIME);
}
Expand Down
74 changes: 71 additions & 3 deletions tests/Unit/Exception/FailureConverterTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,10 @@

namespace Temporal\Tests\Unit\Exception;

use Carbon\CarbonInterval;
use Exception;
use Google\Protobuf\Duration;
use Temporal\Api\Failure\V1\Failure;
use Temporal\DataConverter\DataConverter;
use Temporal\DataConverter\EncodedValues;
use Temporal\Exception\Failure\ApplicationFailure;
Expand All @@ -19,13 +22,13 @@ public function testApplicationFailureCanTransferData(): void
'message',
'type',
true,
EncodedValues::fromValues(['abc', 123])
EncodedValues::fromValues(['abc', 123]),
);

$failure = FailureConverter::mapExceptionToFailure($exception, DataConverter::createDefault());
$restoredDetails = EncodedValues::fromPayloads(
$failure->getApplicationFailureInfo()->getDetails(),
DataConverter::createDefault()
DataConverter::createDefault(),
);

$this->assertSame('abc', $restoredDetails->getValue(0));
Expand Down Expand Up @@ -57,7 +60,7 @@ public function testStackTraceStringForAdditionalContextEvenWhenClassIsNotPresen

try {
$trace = FailureConverter::mapExceptionToFailure(
call_user_func(fn () => new Exception()),
call_user_func(fn() => new Exception()),
DataConverter::createDefault(),
)->getStackTrace();
} finally {
Expand Down Expand Up @@ -104,4 +107,69 @@ public function testStackTraceStringWithoutExceptionArgs(): void
$trace,
);
}

public function testMapFailureToException(): void
{
$converter = DataConverter::createDefault();
$failure = new Failure();
$failure->setApplicationFailureInfo($info = new \Temporal\Api\Failure\V1\ApplicationFailureInfo());
$failure->setStackTrace("test stack trace:\n#1\n#2\n#3");
// Populate the info
$info->setType('testType');
$info->setDetails(EncodedValues::fromValues(['foo', 'bar'], $converter)->toPayloads());
$info->setNonRetryable(true);
$info->setNextRetryDelay((new Duration())->setSeconds(13)->setNanos(15_000));

$exception = FailureConverter::mapFailureToException($failure, $converter);

$this->assertInstanceOf(ApplicationFailure::class, $exception);
$this->assertSame('testType', $exception->getType());
$this->assertTrue($exception->isNonRetryable());
$this->assertSame(['foo', 'bar'], $exception->getDetails()->getValues());
// Next retry delay
$this->assertSame(13, $exception->getNextRetryDelay()->seconds);
$this->assertSame(15, $exception->getNextRetryDelay()->microseconds);
$this->assertTrue($exception->hasOriginalStackTrace());
$this->assertSame("test stack trace:\n#1\n#2\n#3", $exception->getOriginalStackTrace());
}

public function testMapExceptionToFailureWithNextRetryDelay(): void
{
$converter = DataConverter::createDefault();
$exception = new ApplicationFailure(
'message',
'type',
true,
EncodedValues::fromValues(['foo', 'bar'], $converter),
nextRetryDelay: CarbonInterval::fromString('5 minutes 13 seconds 15 microseconds'),
);

$failure = FailureConverter::mapExceptionToFailure($exception, $converter);

$this->assertSame('type', $failure->getApplicationFailureInfo()->getType());
$this->assertTrue($failure->getApplicationFailureInfo()->getNonRetryable());
$this->assertSame(['foo', 'bar'], EncodedValues::fromPayloads(
$failure->getApplicationFailureInfo()->getDetails(),
$converter,
)->getValues());
$this->assertSame(5 * 60 + 13, $failure->getApplicationFailureInfo()->getNextRetryDelay()->getSeconds());
$this->assertSame(15_000, $failure->getApplicationFailureInfo()->getNextRetryDelay()->getNanos());
}

public function testMapExceptionToFailure(): void
{
$converter = DataConverter::createDefault();
$exception = new ApplicationFailure(
'message',
'type',
true,
);

$failure = FailureConverter::mapExceptionToFailure($exception, $converter);

$this->assertSame('type', $failure->getApplicationFailureInfo()->getType());
$this->assertTrue($failure->getApplicationFailureInfo()->getNonRetryable());
$this->assertEmpty($failure->getApplicationFailureInfo()->getDetails());
$this->assertNull($failure->getApplicationFailureInfo()->getNextRetryDelay());
}
}
12 changes: 12 additions & 0 deletions tests/Unit/Internal/Support/DateIntervalTestCase.php
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,18 @@ public function testParseAndFormat(): void
self::assertSame('0/0/0/6', $i->format('%y/%h/%i/%s'));
}

public function testParseFromDuration(): void
{
$duration = (new \Google\Protobuf\Duration())
->setSeconds(5124)
->setNanos(123456000);

$i = DateInterval::parse($duration);

self::assertSame(5124, (int)$i->totalSeconds);
self::assertSame(123_456, $i->microseconds);
}

public static function provideValuesToParse(): iterable
{
yield [1, DateInterval::FORMAT_MICROSECONDS, 1, '0/0/0/0'];
Expand Down
Loading