My analysis was that some elements of $response['data'] were null or not properly initialized. Specifically, when some apps didn’t have matching releases (or, in this case, were not available), $response['data'][$dataKey] = [] is set. However, there may be cases where $response['data'] elements are null or invalid, causing array_filter() to throw an error.

By adding the is_array() check, we should prevent non-array or null values from being passed to array_filter(), which avoids the TypeError. But maybe i'm not seeing it the right way... 👀

No @come-nc's analysis is right:

<?php
array_filter(null);

will give you

PHP Fatal error:  Uncaught TypeError: array_filter(): Argument #1 ($array) must be of type array, null given in /home/jld3103/.config/JetBrains/IntelliJIdea2024.2/scratches/scratch_4.php:2
Stack trace:
#0 /home/jld3103/.config/JetBrains/IntelliJIdea2024.2/scratches/scratch_4.php(2): array_filter()
#1 {main}
  thrown in /home/jld3103/.config/JetBrains/IntelliJIdea2024.2/scratches/scratch_4.php on line 2

Process finished with exit code 255

Which is exactly the same error message. The error message clearly mentions the method and the argument.

Running

<?php
array_filter([null]);

results in no error.

However, there may be cases where $response['data'] elements are null or invalid, causing array_filter() to throw an error.

If no callback is given to array_filter it will remove all falsy values which includes null, so null is not an invalid element in the given array.

Thanks for your inputs, six eyes see better than two 🤣
PR amended.

I wonder if we should have a warning log for the return case to inform the admin that the appstore returned a faulty response? Otherwise it is just silent and they can't know why.

Agree.

Maybe something like:

$this->logger->warning('Response from appstore is invalid, apps could not be retrieved. Try again later.', ['app' => 'appstoreFetcher']);

This should inform admin that response was not valid, but also this could be temporary and it can be retried (in case appstore is down for maintenance, overloaded...).

Sounds good, but I would either omit the app parameter or set it to core.

All the other warnings have it, that's why I've included it. 🤔

Ah ok, then it's good. Can you add it to the PR?

Done ✅

/backport to stable30

/backport to stable29

/backport to stable28