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] Add job timeout occurred event #47068

Merged
merged 3 commits into from
May 16, 2023
Merged

[10.x] Add job timeout occurred event #47068

merged 3 commits into from
May 16, 2023

Conversation

saeedhosseiinii
Copy link
Contributor

@saeedhosseiinii saeedhosseiinii commented May 12, 2023

No event is dispatched when timeout occurss, so there is no way for external packages to know about it. With this pr, this is possible

@nunomaduro nunomaduro marked this pull request as draft May 13, 2023 10:00
@nunomaduro
Copy link
Member

Thanks for your contribution to Laravel! Would you be willing to include some tests in this pull request?

@nunomaduro nunomaduro self-assigned this May 13, 2023
@saeedhosseiinii
Copy link
Contributor Author

Thanks for your contribution to Laravel! Would you be willing to include some tests in this pull request?

To test when the timeout occurred, the process is killed, so the assertion cannot be checked.
To prevent the process from being killed if the kill method of the Worker class is overridden, the Job Processed event is dispatched , but this event is not dispatched in the real scenario.
Is there another way to test?

@nunomaduro
Copy link
Member

@saeedhosseiinii No problem. Testing something as simple as this could be complicated and not worth the hassle. Therefore, let's have Taylor reviewing the pull request without tests.

@nunomaduro nunomaduro marked this pull request as ready for review May 15, 2023 01:33
@taylorotwell
Copy link
Member

You can register an exception handler (ExceptionHandler::reportable method) to handle the new TimeoutExceededException exception. You can do this from your package's service provider.

@saeedhosseiinii
Copy link
Contributor Author

saeedhosseiinii commented May 15, 2023

You can register an exception handler (ExceptionHandler::reportable method) to handle the new TimeoutExceededException exception. You can do this from your package's service provider.

This exception only occurs in the JobFailed event when the job fails.
Consider that I set 5 attempts to the job and timeout occurred on the first attempt. After that the worker is killed and there is no way to know the timeout exception occured!

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.

3 participants