Adjust the processConfigValue check for null, to check on empty instead#190
Adjust the processConfigValue check for null, to check on empty instead#190rjnicolai wants to merge 1 commit intojustbetter:masterfrom
Conversation
| public function processConfigValue(mixed $value, array $config): mixed | ||
| { | ||
| if ($value === null) { | ||
| if (empty($value)) { |
There was a problem hiding this comment.
We probably want to move this check to the specific array type handler (perhaps extract that to a function)
checking empty here will result in the bool config type not functioning as expected.
Since setting that value to false will now always result in null being returned.
There was a problem hiding this comment.
suggestion:
public function processConfigValue(mixed $value, array $config): mixed
{
if ($value === null) {
return null;
}
return match ($config['type']) {
'array' => match (true) {
is_array($value) => $value,
is_string($value) && $value !== '' => $this->serializer->unserialize($value),
default => [],
},
'int' => (int) $value,
'float' => (float) $value,
'bool' => (bool) $value,
'string' => (string) $value,
default => $value,
};
}
There was a problem hiding this comment.
For us, the issue was with "logrocket_key" - are we sure this line is correct and that the key should be an array?
magento2-sentry/Helper/Data.php
Line 76 in ba77329
There was a problem hiding this comment.
You're right, that should indeed not be an array but a string instead.
I'll update an release the change to set that right. After that, feel free to update or close the PR. I Like your suggestion!
Released in 4.4.0 🚀
Summary
Adjust the processConfigValue check for null, to check on empty. Sometimes the value is false which causes the unserialize method to fail.
Result
If empty() evaluates the code will now return to prevent an exception from being triggered because serializer->unserialize() can't process false.
Checklist
composer run codestylecomposer run analyse