sindresorhus / electron-store

Simple data persistence for your Electron app or module - Save and load user preferences, app state, cache, etc
MIT License
4.6k stars 150 forks source link

Schema validation Uncaught Error #116

Open bornova opened 4 years ago

bornova commented 4 years ago

I have a schema which has the following entry:

const schema = {
    dateFormat: {
        type: 'string',
        default: 'l',
        enum: ['l', 'L', 'MMM DD, YYYY', 'ddd, l', 'ddd, L', 'ddd, MMM DD, YYYY']
    }
}

I know config files are not meant to be hand edited, however, if the value of dateFormat is changed to a value outside of what's specified in enum, then an 'Uncaught Error' is thrown in console instead of clearing the config value to its default on app start:

Uncaught Error: Config schema violation: `dateFormat` should be equal to one of the allowed values
    at ElectronStore._validate (..\node_modules\conf\index.js:154)
    at ElectronStore.get store [as store] (..\node_modules\conf\index.js:441)
    at new Conf (..\node_modules\conf\index.js:119)
    at new ElectronStore (..\node_modules\electron-store\index.js:23)
    at test.js:21

So far I have only had this issue when using the enum validation on a string type.

sindresorhus commented 4 years ago

I don't think just clearing the value is a good solution. I think we should rather add some kind of onError hook and if it's not set, we could automatically set the value to the default and also show an error message. That way we have a good default behavior, but apps can override it if needed. I'm open to other ideas.

bornova commented 4 years ago

Hi Sindre, I agree. I was only using the term 'clear' as documented here: clearInvalidConfig which is the default behaviour I believe and it works great except for my specific case. I am only suggesting this default behaviour should also apply to validating a string value against an enum list. Instead, it currently throws the Uncaught Error I stated on my original post.

I also agree some kind of onError hook would be really nice. I currently handle this error by wrapping the const store = new Store({schema}) statement in a try...catch block which is far from perfect.

Nisthar commented 3 years ago

isn't it defaults instead of default?

dertieran commented 3 years ago

I would have also expected that clearInvalidConfig would "clear" the store if the schema validation fails.

What would be the behavior of a onError hook? If it is defined/set the store doesn't throw an error? I guess this should be added to conf? I would still prefer if clearInvalidConfig would also handle this use case, but not sure if there are other implications because of this.

In the meantime my current workaround is using this wrapper

const makeStore = function (options) {
    try {
        return new Store({ ...options });
    } catch (e) {
        const store = new Store({ ...options, schema: null });
        store.clear();
        return return new Store({ ...options });
    }
};