JoliCode\Harvest\Api\Model\Error sometimes returns NULL when it should return a string, triggering an error

[theteknocat]

In my use of the API, I'm checking to see if the result returned by an API request is an instance of the JoliCode\Harvest\Api\Model\Error object and, if so, I call the getMessage method to find out what the error is.

I had wrapped the entire block of code in a try/catch, but it still wouldn't catch and gracefully handle the error that occurs when PHP is unhappy with that method returning null instead of a string, which is specified as the return value in the function signature.

I've updated my code since then to catch \Throwable instead and will see if that takes care of it moving forward, since that will cover either an Error or an Exception thrown by php itself. However, it seems to me that this should be in some way addressed by the error handler in the code so that it just returns an empty string if there is no error message, for whatever reason, available.

Since this code is all generated from the API docs and I'm not familiar with exactly how that works, I'm not sure what the actual solution would be - whether it's doing something to set the $message property of the error object to be an empty string by default, or whatever creates the error object needs to ensure that an empty string is set if and when there is the case of no available error message.

I'm not sure what causes there to be a failure without any message sometimes, as the backtrace provided by PHP doesn't seem to step into all of the functions that are called within the harvest API codebase. Following is an example of the backtrace that I get:

#0 [web-root]/app/Services/Harvest.php(870): JoliCode\Harvest\Api\Model\Error->getMessage()
#1 [web-root]/app/Services/Harvest.php(808): App\Services\Harvest->fetchTimeEntries()
#2 [web-root]/app/Services/Harvest.php(1053): App\Services\Harvest->projectTimeEntries()
#3 [web-root]/app/Console/Commands/SyncHarvestTime.php(64): App\Services\Harvest->syncTimeEntries()
#4 [web-root]/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(36): App\Console\Commands\SyncHarvestTime->handle()
#5 [web-root]/vendor/laravel/framework/src/Illuminate/Container/Util.php(43): Illuminate\Container\BoundMethod::Illuminate\Container\{closure}()
#6 [web-root]/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(95): Illuminate\Container\Util::unwrapIfClosure()
#7 [web-root]/vendor/laravel/framework/src/Illuminate/Container/BoundMethod.php(35): Illuminate\Container\BoundMethod::callBoundMethod()
#8 [web-root]/vendor/laravel/framework/src/Illuminate/Container/Container.php(694): Illuminate\Container\BoundMethod::call()
#9 [web-root]/vendor/laravel/framework/src/Illuminate/Console/Command.php(213): Illuminate\Container\Container->call()
#10 [web-root]/vendor/symfony/console/Command/Command.php(279): Illuminate\Console\Command->execute()
#11 [web-root]/vendor/laravel/framework/src/Illuminate/Console/Command.php(182): Symfony\Component\Console\Command\Command->run()
#12 [web-root]/vendor/symfony/console/Application.php(1094): Illuminate\Console\Command->run()
#13 [web-root]/vendor/symfony/console/Application.php(342): Symfony\Component\Console\Application->doRunCommand()
#14 [web-root]/vendor/symfony/console/Application.php(193): Symfony\Component\Console\Application->doRun()
#15 [web-root]/vendor/laravel/framework/src/Illuminate/Foundation/Console/Kernel.php(198): Symfony\Component\Console\Application->run()
#16 [web-root]/artisan(35): Illuminate\Foundation\Console\Kernel->handle()
#17 {main}

I'm not sure why PHP's Exception::getTraceAsString() doesn't include all the function calls within the harvest API object in between the call in my Harvest service class to fetch the time entries and it's subsequent call to get the message from the returned error object. I'll see if that takes some additional arguments to get more detail in the trace, but for now I hope this is enough to go on.

Hopefully you can find a way to test the code and simulate an error with no message to determine the best course of action to rectify this. If I can get more details (such as a more detailed backtrace) or anything more helpful, I'll post it to this issue thread.

[xavierlacot]

Hi @theteknocat

It seems that Jane was not correctly typehinting nullable UserAssignment and TaskAssignment properties in the Expense and TimeEntry models. I changed the was nullable sub-models are defined in the OpenAPI spec (see jolicode/harvest-openapi-generator#30) and re-generated the library's code.

Can you please update to v8.0.0 and check if it now works correctly in your case?

[theteknocat]

Hi @xavierlacot

So far so good, that seems to have done the trick. I removed all the try/catch statements around the call to getMessage when the result is the error object and after running my harvest sync script a several times locally, it didn't run into the same error.

However, I don't think I was ever able to recreate the issue on my local, so I'm deploying the changes to production and will see if it sends any further error report emails for this specifically when it runs the cron job for synchronizing Harvest time with the application.

[xavierlacot]

Sounds good. Please let me know if the issue can be closed 😃

[xavierlacot]

Hi @theteknocat Did the last update fix the issue? If yes, can you please close the issue? 🙏

[xavierlacot]

Closing the issue, must be fixed.