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

FIX for #11166 Index Handling Fatal Error #11183

Merged
merged 2 commits into from
Oct 12, 2017
Merged

FIX for #11166 Index Handling Fatal Error #11183

merged 2 commits into from
Oct 12, 2017

Conversation

larsroettig
Copy link
Member

@larsroettig larsroettig commented Oct 2, 2017

see #11166

  • Add Error Handling
  • Implement Test Case

- Add Error Handling
- Impelement Test Case
@@ -418,6 +418,11 @@ public function reindexAll()
$state->save();
$this->getView()->resume();
throw $exception;
}catch (\Error $error) {
Copy link
Contributor

Choose a reason for hiding this comment

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

  • please add space before catch
  • is there more specific exception you can catch?

Copy link
Member Author

Choose a reason for hiding this comment

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

It not so easy to a more specific exception:
Currently, php7 throw much different but you must catch all to set the correct state for the indexer. After a fatal, the indexer must be on state invalid

php__errors_in_php_7_-_manual

@vrann vrann self-assigned this Oct 2, 2017
@vrann vrann added this to the October 2017 milestone Oct 2, 2017
@okorshenko okorshenko merged commit 214fc33 into magento:2.2-develop Oct 12, 2017
@hostep
Copy link
Contributor

hostep commented Oct 13, 2017

Nice, thanks @larsroettig!

I think this fix should be backported to Magento 2.1.x, would be appreciated if someone is willing to do this.

@larsroettig
Copy link
Member Author

Hi, @hostep I cannot so easy backport my Fix because 2.1.X allowed to use php5.6 and this Error handling requires php7.0 it is broken change for 2.1.X. To catch Engine Error only for one function is in php5.6 not so simple.

Magento 2.1.X:

"php": "~5.6.5|7.0.2|7.0.4|~7.0.6",

Magento 2.2.X

"php": "7.0.2|7.0.4|~7.0.6|~7.1.0",

@hostep
Copy link
Contributor

hostep commented Oct 21, 2017

@larsroettig: ah yes, you have a valid point there!

We could add a dependency to https://github.com/symfony/polyfill-php70 to get around this, but that's probably a bit out of scope and might introduce new problems.

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

Successfully merging this pull request may close these issues.

6 participants