Level: cleaned up chunk loading error handling, close #2056

This now removes logging from the level providers (for the most part) and replaces it with exception throws and catches. The implementation using the providers should catch these exceptions if they are thrown.

src/pocketmine/level/Level.php

@@ -2706,16 +2706,24 @@ class Level implements ChunkManager, Metadatable{
 
 		$this->timings->syncChunkLoadDataTimer->startTiming();
 
-		$chunk = $this->provider->loadChunk($x, $z, $create);
+		$chunk = null;
+
+		try{
+			$chunk = $this->provider->loadChunk($x, $z);
+		}catch(\Exception $e){
+			$logger = $this->server->getLogger();
+			$logger->critical("An error occurred while loading chunk x=$x z=$z: " . $e->getMessage());
+			$logger->logException($e);
+		}
+
+		if($chunk === null and $create){
+			$chunk = new Chunk($x, $z);
+		}
 
 		$this->timings->syncChunkLoadDataTimer->stopTiming();
 
 		if($chunk === null){
 			$this->timings->syncChunkLoadTimer->stopTiming();
-
-			if($create){
-				throw new \InvalidStateException("Could not create new Chunk");
-			}
 			return false;
 		}
 

src/pocketmine/level/format/io/BaseLevelProvider.php

@@ -113,13 +113,8 @@ abstract class BaseLevelProvider implements LevelProvider{
 		file_put_contents($this->getPath() . "level.dat", $buffer);
 	}
 
-	public function loadChunk(int $chunkX, int $chunkZ, bool $create = false) : ?Chunk{
-		$chunk = $this->readChunk($chunkX, $chunkZ);
-		if($chunk === null and $create){
-			$chunk = new Chunk($chunkX, $chunkZ);
-		}
-
-		return $chunk;
+	public function loadChunk(int $chunkX, int $chunkZ) : ?Chunk{
+		return $this->readChunk($chunkX, $chunkZ);
 	}
 
 	public function saveChunk(Chunk $chunk) : void{

src/pocketmine/level/format/io/LevelProvider.php

@@ -93,16 +93,17 @@ interface LevelProvider{
 	public function saveChunk(Chunk $chunk) : void;
 
 	/**
-	 * Loads a chunk (usually from disk storage) and returns it. If the chunk does not exist, null is returned, or an
-	 * empty Chunk if $create is specified.
+	 * Loads a chunk (usually from disk storage) and returns it. If the chunk does not exist, null is returned.
 	 *
 	 * @param int $chunkX
 	 * @param int $chunkZ
-	 * @param bool $create
 	 *
 	 * @return null|Chunk
+	 *
+	 * @throws \Exception any of a range of exceptions that could be thrown while reading chunks. See individual
+	 * implementations for details.
 	 */
-	public function loadChunk(int $chunkX, int $chunkZ, bool $create = false) : ?Chunk;
+	public function loadChunk(int $chunkX, int $chunkZ) : ?Chunk;
 
 	/**
 	 * @return string

src/pocketmine/level/format/io/exception/CorruptedChunkException.php

@@ -0,0 +1,30 @@
+<?php
+
+/*
+ *
+ *  ____            _        _   __  __ _                  __  __ ____
+ * |  _ \ ___   ___| | _____| |_|  \/  (_)_ __   ___      |  \/  |  _ \
+ * | |_) / _ \ / __| |/ / _ \ __| |\/| | | '_ \ / _ \_____| |\/| | |_) |
+ * |  __/ (_) | (__|   <  __/ |_| |  | | | | | |  __/_____| |  | |  __/
+ * |_|   \___/ \___|_|\_\___|\__|_|  |_|_|_| |_|\___|     |_|  |_|_|
+ *
+ * This program is free software: you can redistribute it and/or modify
+ * it under the terms of the GNU Lesser General Public License as published by
+ * the Free Software Foundation, either version 3 of the License, or
+ * (at your option) any later version.
+ *
+ * @author PocketMine Team
+ * @link http://www.pocketmine.net/
+ *
+ *
+*/
+
+declare(strict_types=1);
+
+namespace pocketmine\level\format\io\exception;
+
+use pocketmine\level\format\ChunkException;
+
+class CorruptedChunkException extends ChunkException{
+
+}

src/pocketmine/level/format/io/leveldb/LevelDB.php

@@ -39,7 +39,6 @@ use pocketmine\nbt\tag\{
 use pocketmine\network\mcpe\protocol\ProtocolInfo;
 use pocketmine\utils\Binary;
 use pocketmine\utils\BinaryStream;
-use pocketmine\utils\MainLogger;
 
 class LevelDB extends BaseLevelProvider{
 
@@ -263,6 +262,7 @@ class LevelDB extends BaseLevelProvider{
 	 * @param int $chunkZ
 	 *
 	 * @return Chunk|null
+	 * @throws UnsupportedChunkFormatException
 	 */
 	protected function readChunk(int $chunkX, int $chunkZ) : ?Chunk{
 		$index = LevelDB::chunkIndex($chunkX, $chunkZ);
@@ -271,7 +271,6 @@ class LevelDB extends BaseLevelProvider{
 			return null;
 		}
 
-		try{
 		/** @var SubChunk[] $subChunks */
 		$subChunks = [];
 
@@ -312,6 +311,7 @@ class LevelDB extends BaseLevelProvider{
 							$subChunks[$y] = new SubChunk($blocks, $blockData, $blockSkyLight, $blockLight);
 							break;
 						default:
+							//TODO: set chunks read-only so the version on disk doesn't get overwritten
 							throw new UnsupportedChunkFormatException("don't know how to decode LevelDB subchunk format version $subChunkVersion");
 					}
 				}
@@ -360,6 +360,7 @@ class LevelDB extends BaseLevelProvider{
 				$biomeIds = ChunkUtils::convertBiomeColors(array_values(unpack("N*", $binaryStream->get(1024))));
 				break;
 			default:
+				//TODO: set chunks read-only so the version on disk doesn't get overwritten
 				throw new UnsupportedChunkFormatException("don't know how to decode chunk format version $chunkVersion");
 		}
 
@@ -418,21 +419,6 @@ class LevelDB extends BaseLevelProvider{
 		$chunk->setLightPopulated($lightPopulated);
 
 		return $chunk;
-		}catch(UnsupportedChunkFormatException $e){
-			//TODO: set chunks read-only so the version on disk doesn't get overwritten
-
-			$logger = MainLogger::getLogger();
-			$logger->error("Failed to decode LevelDB chunk: " . $e->getMessage());
-
-			return null;
-		}catch(\Throwable $t){
-			$logger = MainLogger::getLogger();
-			$logger->error("LevelDB chunk decode error");
-			$logger->logException($t);
-
-			return null;
-
-		}
 	}
 
 	protected function writeChunk(Chunk $chunk) : void{

src/pocketmine/level/format/io/region/Anvil.php

@@ -33,7 +33,6 @@ use pocketmine\nbt\tag\ByteArrayTag;
 use pocketmine\nbt\tag\CompoundTag;
 use pocketmine\nbt\tag\IntArrayTag;
 use pocketmine\nbt\tag\ListTag;
-use pocketmine\utils\MainLogger;
 
 class Anvil extends McRegion{
 
@@ -99,7 +98,6 @@ class Anvil extends McRegion{
 
 	protected function nbtDeserialize(string $data){
 		$nbt = new BigEndianNBTStream();
-		try{
 		$chunk = $nbt->readCompressed($data);
 		if(!($chunk instanceof CompoundTag) or !$chunk->hasTag("Level")){
 			throw new ChunkException("Invalid NBT format");
@@ -134,10 +132,6 @@ class Anvil extends McRegion{
 		$result->setPopulated($chunk->getByte("TerrainPopulated", 0) !== 0);
 		$result->setGenerated();
 		return $result;
-		}catch(\Throwable $e){
-			MainLogger::getLogger()->logException($e);
-			return null;
-		}
 	}
 
 	protected function deserializeSubChunk(CompoundTag $subChunk) : SubChunk{

src/pocketmine/level/format/io/region/McRegion.php

@@ -111,7 +111,6 @@ class McRegion extends BaseLevelProvider{
 	 */
 	protected function nbtDeserialize(string $data){
 		$nbt = new BigEndianNBTStream();
-		try{
 		$chunk = $nbt->readCompressed($data);
 		if(!($chunk instanceof CompoundTag) or !$chunk->hasTag("Level")){
 			throw new ChunkException("Invalid NBT format");
@@ -181,10 +180,6 @@ class McRegion extends BaseLevelProvider{
 		$result->setPopulated($chunk->getByte("TerrainPopulated", 0) !== 0);
 		$result->setGenerated(true);
 		return $result;
-		}catch(\Throwable $e){
-			MainLogger::getLogger()->logException($e);
-			return null;
-		}
 	}
 
 	public static function getProviderName() : string{

src/pocketmine/level/format/io/region/RegionLoader.php

@@ -24,6 +24,7 @@ declare(strict_types=1);
 namespace pocketmine\level\format\io\region;
 
 use pocketmine\level\format\ChunkException;
+use pocketmine\level\format\io\exception\CorruptedChunkException;
 use pocketmine\utils\Binary;
 use pocketmine\utils\MainLogger;
 
@@ -95,10 +96,18 @@ class RegionLoader{
 		return !($this->locationTable[$index][0] === 0 or $this->locationTable[$index][1] === 0);
 	}
 
+	/**
+	 * @param int $x
+	 * @param int $z
+	 *
+	 * @return null|string
+	 * @throws \InvalidArgumentException if invalid coordinates are given
+	 * @throws CorruptedChunkException if chunk corruption is detected
+	 */
 	public function readChunk(int $x, int $z) : ?string{
 		$index = self::getChunkOffset($x, $z);
 		if($index < 0 or $index >= 4096){
-			return null;
+			throw new \InvalidArgumentException("Invalid chunk position in region, expected x/z in range 0-31, got x=$x, z=$z");
 		}
 
 		$this->lastUsed = time();
@@ -115,27 +124,26 @@ class RegionLoader{
 			if($length >= self::MAX_SECTOR_LENGTH){
 				$this->locationTable[$index][0] = ++$this->lastSector;
 				$this->locationTable[$index][1] = 1;
-				MainLogger::getLogger()->error("Corrupted chunk header detected");
+				throw new CorruptedChunkException("Corrupted chunk header detected (sector count larger than max)");
 			}
 			return null;
 		}
 
 		if($length > ($this->locationTable[$index][1] << 12)){ //Invalid chunk, bigger than defined number of sectors
-			MainLogger::getLogger()->error("Corrupted bigger chunk detected");
+			MainLogger::getLogger()->error("Corrupted bigger chunk detected (bigger than number of sectors given in header)");
 			$this->locationTable[$index][1] = $length >> 12;
 			$this->writeLocationIndex($index);
 		}elseif($compression !== self::COMPRESSION_ZLIB and $compression !== self::COMPRESSION_GZIP){
-			MainLogger::getLogger()->error("Invalid compression type");
-			return null;
+			throw new CorruptedChunkException("Invalid compression type (got $compression, expected " . self::COMPRESSION_ZLIB . " or " . self::COMPRESSION_GZIP . ")");
 		}
 
 		$chunkData = fread($this->filePointer, $length - 1);
-		if($chunkData !== false){
-			return $chunkData;
-		}else{
-			MainLogger::getLogger()->error("Corrupted chunk detected");
-			return null;
+		if($chunkData === false){
+			throw new CorruptedChunkException("Corrupted chunk detected (failed to read chunk data from disk)");
+
 		}
+
+		return $chunkData;
 	}
 
 	public function chunkExists(int $x, int $z) : bool{