rawilk / laravel-settings

Store Laravel application settings in the database.
https://randallwilk.dev/docs/laravel-settings
MIT License
199 stars 17 forks source link

Getting unserialize(): Error at offset 0 of 228 bytes: on log file #39

Closed ichie-benjamin closed 1 year ago

ichie-benjamin commented 1 year ago

Laravel Settings

v2.2

Laravel Version

10

Bug description

This happens on laravel 10 using uuid as primary key

On log file i get unserialize(): Error at offset 0 of 228 bytes

[2023-10-04 11:23:07] local.ERROR: unserialize(): Error at offset 0 of 228 bytes {"userId":"9a2a4708-40f8-4384-98d9-00d54d7293b1","exception":"[object] (ErrorException(code: 0): unserialize(): Error at offset 0 of 228 bytes at /vendor/rawilk/laravel-settings/src/Support/ValueSerializer.php:16) [stacktrace]

6 /vendor/laravel/framework/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php(254): Illuminate\Foundation\Bootstrap\HandleExceptions->handleError(8, 'unserialize(): ...', '/Do...', 16)

settings are not user-based, am not making a setting for users, so don't knw why the userid

Steps to reproduce

Install in laravel 10 use UUID as primary key create a user and login

accessing any page throws the error in log file

Relevant log output

[2023-10-04 11:23:07] local.ERROR: unserialize(): Error at offset 0 of 228 bytes {"userId":"9a2a4708-40f8-4384-98d9-00d54d7293b1","exception":"[object] (ErrorException(code: 0): unserialize(): Error at offset 0 of 228 bytes at /vendor/rawilk/laravel-settings/src/Support/ValueSerializer.php:16)
[stacktrace]
#0 /vendor/laravel/framework/src/Illuminate/Foundation/Bootstrap/HandleExceptions.php(254): Illuminate\\Foundation\\Bootstrap\\HandleExceptions->handleError(8, 'unserialize(): ...', '/Users/benny/Do...', 16)
#1 [internal function]: Illuminate\\Foundation\\Bootstrap\\HandleExceptions->Illuminate\\Foundation\\Bootstrap\\{closure}(8, 'unserialize(): ...', '/Do...', 16)
#2 /vendor/rawilk/laravel-settings/src/Support/ValueSerializer.php(16): unserialize('eyJpdiI6ImMrT1V...')
#3 /vendor/rawilk/laravel-settings/src/Settings.php(203): Rawilk\\Settings\\Support\\ValueSerializer->unserialize('eyJpdiI6ImMrT1V...')
rawilk commented 1 year ago

It appears that the user id is just given for context in this exception and is not part of the issue. The actual exception is that there's an issue with unserialize being used on your setting value. I've used v2 of this package in projects that use uuids as primary keys, so I know that's not really an issue with it.

Also, if you can, I'd recommend upgrading to v3 of this package, as it offers more flexibility on how settings are stored and retrieved.

Hope this helps.

ichie-benjamin commented 1 year ago

I have upgraded to v3 same error

rawilk commented 1 year ago

Are you making any calls to settings()->get()?

ichie-benjamin commented 1 year ago

no, i am not

settings('site_slogan') same for other uses

I think i found the issue saving an array as setting value logs the error, and keep logging on every page even with version 3

rawilk commented 1 year ago

settings('site_slogan') will make a call to get for you, which will in turn use the default ValueSerializer to unserialize the stored value.

I'm pretty sure storing arrays should be supported, however I have not verified this myself. I may look into that later, however if you're able to find an issue with it and PR a fix with tests, I'd also appreciate that.

MCKLtech commented 1 year ago

I'm getting the same error on the newest release.

I also get weird behaviour, for example:

function get_users_fav_food() {

    $fav_food = $user->settings()->get('fav_food', 'pasta');
    //More code
    $fav_food = $user->settings()->get('fav_food', 'pizza');

    return $fav_food
}

Even if I don't call set, the return value of $fav_food is pasta when it should be pizza as that was the last default passed. Does the default get saved/cached automatically?

rawilk commented 1 year ago

@MCKLtech The default does get cached automatically when caching is enabled, so the first value you provide as the default will be returned until you bust the cache for that setting by either saving a value to it or clearing it.

I'm not really sure why this error would be happening. I would make sure that your config file is the same as the default for the package, and to follow the upgrade guide if you're upgrading. I'm not sure when I'll have a chance to look further into this, but I'll try to replicate this if I can at some point.

MCKLtech commented 1 year ago

Ok, do you know if that caching behaviour is documented? If not, it should be, as I expect it to work differently. If I call get(), I expect whatever default I specify to be returned unless it's already been set/saved. The setting should always return the specified default if it has not previously been saved, otherwise I assume I need to constantly flush?

MCKLtech commented 1 year ago

Quick follow up, republishing the settings config appears to have solved this and also seems to have fixed the weird get() behaviour. I'll keep testing.

However, I still believe that get() should always return the specified default if the setting has not been previously saved/set

rawilk commented 1 year ago

I don't think I have that specific caching behavior documented, but I can certainly add it in to the docs.

One way around that is to temporarily disable the cache if you're sure a setting is not set yet.

settings()->temporarilyDisableCache()->get(...);

This typically isn't the use case for that method, but it could help. I usually use that in queued jobs when I don't necessarily need the cache.

rawilk commented 1 year ago

However, I still believe that get() should always return the specified default if the setting has not been previously saved/set

I agree; it's just not something I really thought of. Right now, this is currently how the value is retrieved when caching is enabled:

if ($this->cacheIsEnabled()) {
    $value = $this->cache->rememberForever(
        $this->getCacheKey($generatedKey),
        fn () => $this->driver->get(
            key: $generatedKey,
            default: $default,
            teamId: $this->teams ? $this->teamId : false,
        )
    );
} else {
    $value = $this->driver->get(
        key: $generatedKey,
        default: $default,
        teamId: $this->teams ? $this->teamId : false,
    );
}

See: https://github.com/rawilk/laravel-settings/blob/main/src/Settings.php#L152C9-L167C10

Maybe instead of returning a default value from the driver, I could just omit that entirely and only rely on the default provided to the settings service. That way the default value is not included in the cache call.

MCKLtech commented 1 year ago

Maybe make it configurable? I can see some narrow use cases where a default could be cached, but in my instance, it lead to quite a bit of confusion as I always assume a default is as specified, not what could have been previously specified.

rawilk commented 1 year ago

The issues regarding the cached default value functionality have been addressed in v3.1.2. As far as the original issue goes, I think it's just a matter of the configuration file not being up-to-date.

Feel free to re-open the issue if there's still an issue with it.