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

Swoole & Octane::concurrently undefined array key on timed out task results #395

Closed
sybbear opened this issue Oct 5, 2021 · 4 comments · Fixed by #396
Closed

Swoole & Octane::concurrently undefined array key on timed out task results #395

sybbear opened this issue Oct 5, 2021 · 4 comments · Fixed by #396
Labels

Comments

@sybbear
Copy link
Contributor

sybbear commented Oct 5, 2021

  • Octane Version: 1.0.12
  • Laravel Version: 8.62.0
  • PHP Version: 8.0.0
  • Server & Version: Swoole 4.7.1
  • Database Driver & Version: -

Description:

Swoole's taskWaitMulti omits returning results, tasks of which have timed out.
Check the information here: swoole/swoole-src#4423

Then a chunk of code in concurrently method causes an ErrorException (undefined array key x) and technically crashes the whole result fetching process.

https://github.com/laravel/octane/blob/1.x/src/Swoole/SwooleTaskDispatcher.php#L46

foreach ($tasks as $key => $task) {
    if ($results[$i] instanceof TaskExceptionResult) { // ErrorException Undefined array key
        throw $results[$i]->getOriginal();
    }

Steps To Reproduce:

Queue a set of tasks with different execution times in such a way, that at least one of them times out.
Not having enough workers could be a reason for timeout.

Resolution suggestion:

Return check that the key exists and if not, assign false as a result array element.

@driesvints
Copy link
Member

Looks like a straightforward fix. Can you send in a PR?

@sybbear
Copy link
Contributor Author

sybbear commented Oct 5, 2021

Added a PR, however cannot delve into the tests and styling as we have priority projects going on right now.
Hopefully this helps to release a fix.

@driesvints
Copy link
Member

I fixed em for you 👍

@sybbear
Copy link
Contributor Author

sybbear commented Oct 5, 2021

Ah, with styling there were a bunch of spaces in front of new line. Got it, thank you 😎

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

Successfully merging a pull request may close this issue.

2 participants