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

[10.x] Remember the job on the exception #48830

Merged
merged 5 commits into from
Oct 30, 2023

Conversation

timacdonald
Copy link
Member

@timacdonald timacdonald commented Oct 27, 2023

When these exceptions are triggered they pass through the applications handlers reporting piping.

It is be common for a certain type of job to be okay to timeout and be treated "special", e.g., log it but not to a 3rd party error monitoring.

Currently the only way to do this is to check the error message, .e.g.,

if (
    $e instanceof MaxAttemptsExceededException &&
    str_starts_with($e->getMessage(), MySpecialJob::class.' ')
) {
    //
}

While not the end of the world, it feels a flaky and yuck. This exposes the job on the exception.

if (
    $e instanceof MaxAttemptsExceededException &&
    $e->job->getResolvedJob() instanceof MySpecialJob
) {
    //
}

tests/Queue/QueueExceptionTest.php Outdated Show resolved Hide resolved
tests/Queue/QueueExceptionTest.php Outdated Show resolved Hide resolved
driesvints and others added 2 commits October 27, 2023 09:50
Co-authored-by: Julius Kiekbusch <jubeki99@gmail.com>
Co-authored-by: Julius Kiekbusch <jubeki99@gmail.com>
@driesvints
Copy link
Member

@timacdonald looks good. What are your thoughts on the job having a large payload and thus filling up logs when being reported?

@timacdonald
Copy link
Member Author

@driesvints my understanding is that custom properties on exceptions are ignored when reporting or logging.

I've never seen attaching a property to an exception class cause any issues.

See also: #47223 (comment)

@taylorotwell taylorotwell merged commit 9d7868b into laravel:10.x Oct 30, 2023
19 of 20 checks passed
@timacdonald timacdonald deleted the remember-job branch October 30, 2023 00:32
@driesvints
Copy link
Member

@timacdonald yup, you're totally right! 👍

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.

None yet

4 participants