Pocketmine-mp: Abuse of catch(Throwable)

Created on 1 Mar 2018  路  2Comments  路  Source: pmmp/PocketMine-MP

Issue description

Since PHP 7, many things that were previously fatal errors in PHP 5 are now catchable thrown Error objects. The Throwable interface is implemented by both Error and Exception to allow what should be a rare catch-all-things case, for example for logging or crashdumps.

PocketMine-MP heavily abuses catch(\Throwable) since PHP 7, which results in many things that ought to cause crashdumps, to produce logged exceptions and then attempt to continue. This is bad behaviour. catch(\Throwable) should only be used in a handful of places - most other things should only be catching Exceptions. Errors are usually thrown to indicate problems with user code, _not_ runtime errors.

As seen here, Errors were explicitly segregated from Exceptions to _maintain backwards compatibility_ to some degree, and prevent existing catches catching Errors. Catch-all everywhere is a bad thing.

Steps to reproduce the issue

  1. trigger an Error somewhere down the line
  2. behold the attempt to continue execution wrongfully on a case that should cause a crashdump to be emitted.

OS and versions

  • PocketMine-MP: any since PHP 7
  • PHP: 7.any
Core Fixed

Most helpful comment

A quick example:

//assume we are loading a chunk
try{
    $chunk = $this->provider->loadChunk($chunkX, $chunkZ); //we forgot to set chunkZ to a number
}catch(\Throwable $e){
    $this->server->getLogger()->error("Corrupted chunk detected: " . $e->getMessage()); //this isn't a corrupted chunk, it's a TypeError due to defective code!
}

All 2 comments

A quick example:

//assume we are loading a chunk
try{
    $chunk = $this->provider->loadChunk($chunkX, $chunkZ); //we forgot to set chunkZ to a number
}catch(\Throwable $e){
    $this->server->getLogger()->error("Corrupted chunk detected: " . $e->getMessage()); //this isn't a corrupted chunk, it's a TypeError due to defective code!
}

This has been fully addressed on master as of 48a99937b9556d0b04121d58ea45dce994d88633.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

kenygamer picture kenygamer  路  92Comments

zKoz210 picture zKoz210  路  23Comments

MisteFr picture MisteFr  路  29Comments

MisteFr picture MisteFr  路  18Comments

VCraftMCPE picture VCraftMCPE  路  20Comments