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

BUGFIX: When working on job, catch Throwable #59

Merged
merged 1 commit into from
Jul 28, 2022

Conversation

kdambekalns
Copy link
Member

When working on a job, any \Exception is caught and the job in the
queue is released (retry or failure). In case of a \Throwable not
being an \Exception, this does not happen and the job is never "un-
reserved".

This change catches \Throwable instead to fix this.

When working on a job, any `\Exception` is caught and the job in the
queue is released (retry or failure). In case of a `\Throwable` _not_
being an `\Exception`, this does not happen and the job is never "un-
reserved".

This change catches `\Throwable` instead to fix this.
@kdambekalns kdambekalns self-assigned this Jul 28, 2022
Copy link
Contributor

@bwaidelich bwaidelich left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you!

@bwaidelich bwaidelich merged commit 87b5738 into Flowpack:master Jul 28, 2022
@kdambekalns kdambekalns deleted the bugfix/catch-throwable branch July 28, 2022 09:30
$maximumNumberOfReleases = isset($queueSettings['maximumNumberOfReleases']) ?
(int)$queueSettings['maximumNumberOfReleases'] :
self::DEFAULT_MAXIMUM_NUMBER_RELEASES;
if ($message->getNumberOfReleases() < $maximumNumberOfReleases) {
$releaseOptions = isset($queueSettings['releaseOptions']) ? $queueSettings['releaseOptions'] : [];
$queue->release($message->getIdentifier(), $releaseOptions);
$this->emitMessageReleased($queue, $message, $releaseOptions, $exception);
$logMessage = $this->throwableStorage->logThrowable($exception);
$this->emitMessageReleased($queue, $message, $releaseOptions, $throwable);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doh, this part now fails if $throwable is an \Error – and changing the signal signature would be a breaking change

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

bwaidelich added a commit that referenced this pull request Jul 28, 2022
This is a follow-up for #59 that adjusts the signatures of
`JobManager::emitMessageReleased()` and `JobManager::emitMessageFailed()`
to avoid type errors when invoking the methods.

Related: #59
bwaidelich added a commit that referenced this pull request Jul 28, 2022
This is a follow-up for #59 that adjusts the signatures of
`JobManager::emitMessageReleased()` and `JobManager::emitMessageFailed()`
to avoid type errors when invoking the methods.

Related: #59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants