PluginManager: Stop catching exceptions thrown by event handlers (#2472)

The basic principle here is "if you're not expecting it, don't catch it".

Event handlers are **never** supposed to throw exceptions. If they do throw exceptions, it's usually going to one of two things;
1. Broken code producing an error
2. Code triggering (and not catching) a runtime error

Both 1) and 2) boil down to defective code on the part of the event handler, and thus should not be caught by the caller, but instead allowed to crash the server and produce a crashdump.

It's also undesirable to catch unexpected errors here for a few other reasons
- It leaves the owner of the event handler in an unknown, potentially unstable state
- It allows broken code to cause event handlers to spam the logger in events that happen frequently (for example movement handlers)
- It allows the process to continue down a train of further undefined behaviour, which may lead to more errors or ultimately a crash, so it makes no sense to hold off the inevitable.

This has a few advantages that are not merely inverted disadvantages:
- Crash dumps will now be created and automatically submitted for defective event handlers, allowing quicker issue location, debugging and fixing in plugins without manual user interaction
- Event calling now isn't dependent on Server to work.
This commit is contained in:
Dylan K. Taylor 2018-10-05 16:46:10 +01:00 committed by GitHub
parent c065cfbeda
commit b1e0f82cbf
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23

View File

@ -698,18 +698,7 @@ class PluginManager{
continue;
}
try{
$registration->callEvent($event);
}catch(\Throwable $e){
$this->server->getLogger()->critical(
$this->server->getLanguage()->translateString("pocketmine.plugin.eventError", [
$event->getEventName(),
$registration->getPlugin()->getDescription()->getFullName(),
$e->getMessage(),
get_class($registration->getListener())
]));
$this->server->getLogger()->logException($e);
}
$registration->callEvent($event);
}
$currentList = $currentList->getParent();