null value in Argument 1 passed to setServerGeoLocation

[markkrueg]

I am receiving this crash several times per day:

Error: Argument 1 passed to EssentialsPE\BaseFiles\BaseAPI::setServerGeoLocation() must be of the type string, null given, called in phar:///var/games/minecraft/servers/test/plugins/EssentialsPE_v2.0.0.phar/src/EssentialsPE/Tasks/GeoLocation.php on line 59
File: /EssentialsPE_v2.0.0.phar/src/EssentialsPE/BaseFiles/BaseAPI
Line: 729
Type: notice

THIS CRASH WAS CAUSED BY A PLUGIN
BAD PLUGIN : EssentialsPE v2.0.0

Code:
[720]      * @param string $location
[721]      */
[722]     public function updateGeoLocation(Player $player, string $location){
[723]         $this->getSession($player)->setGeoLocation($location);
[724]     }
[725] 
[726]     /**
[727]      * @param string $location
[728]      */
[729]     public function setServerGeoLocation(string $location){
[730]         if($this->serverGeoLocation === null){
[731]             $this->serverGeoLocation = $location;
[732]         }
[733]     }
[734] 
[735]     /**   _____           _
[736]      *   / ____|         | |
[737]      *  | |  __  ___   __| |
[738]      *  | | |_ |/ _ \ / _` |
[739]      *  | |__| | (_) | (_| |

Backtrace:
#0 /src/pocketmine/scheduler/AsyncPool(151): EssentialsPE\Tasks\GeoLocation->onCompletion(pocketmine\Server object)
#1 /src/pocketmine/scheduler/ServerScheduler(268): pocketmine\scheduler\AsyncPool->collectTasks(boolean)
#2 /src/pocketmine/Server(2945): pocketmine\scheduler\ServerScheduler->mainThreadHeartbeat(integer 231)
#3 /src/pocketmine/Server(2641): pocketmine\Server->tick(boolean)
#4 /src/pocketmine/Server(2519): pocketmine\Server->tickProcessor(boolean)
#5 /src/pocketmine/Server(2052): pocketmine\Server->start(boolean)
#6 /src/pocketmine/PocketMine(467): pocketmine\Server->__construct(pocketmine\CompatibleClassLoader object, pocketmine\utils\MainLogger object, string phar:///var/games/minecraft/servers/test/PocketMine-MP.phar/, string /var/games/minecraft/servers/test/, string /var/games/minecraft/servers/test/plugins/, string unknown)
#7 (1): require_once(string phar:///var/games/minecraft/servers/test/PocketMine-MP.phar/src/pocketmine/PocketMine.php)

[markkrueg]

I'm trying to further debug this, as it caused my server to crash 13 times today alone.

If I am understanding the crash log; the problem is that a null value is being passed in line 59 of GeoLocation.php to the setServerGeoLocation public function.

Looking at this function, it is crashing here:

public function setServerGeoLocation(string $location){

I assume there is a problem using "string $location" when $location is null?

How would I avoid ever sending a null here?

    public function onCompletion(Server $server){
        /** @var Loader $plugin */
        $plugin = $server->getPluginManager()->getPlugin("EssentialsPE");
        if(!is_array($this->getResult())){
            $plugin->getAPI()->setServerGeoLocation($this->getResult());
        }else{
            foreach($this->getResult() as $spl => $loc){
                $plugin->getAPI()->updateGeoLocation($this->player[$spl], ($loc !== "server" ?? $plugin->getAPI()->getServerGeoLocation()));
            }
        }

Could I enclose this code section with a test for null?

Sorry, I'm not really much of a PHP coder...any help would be much appreciated.

[PEMapModder]

The setter method needs to be updated to accept null. This can be done by adding = null as default in the method declaration. @iksaku

[markkrueg]

Thanks @PEMapModder So basically line 729 should look like:

[729]     public function setServerGeoLocation(string $location = null){

I will experiment with this.

[markkrueg]

Pull request submitted. I have not seen a crash since this change.

[markkrueg]

Reference: This works because:

"The declaration can be made to accept NULL values if the default value of the parameter is set to NULL."

as written here:

http://php.net/manual/en/functions.arguments.php

[PEMapModder]

@markkrueg Yes, thank you. Just waiting for @iksaku to merge it when he has finished his current commit.

[iksaku]

I do have a fix for this locally, but allowing NULL would create a warning when "getting" the location, since that other function expects string to be returned... I will not merge it and I'll try to push the commit today