Add error hook option

[jnummelin]

I was playing with migrations and noticed some weird behavior. So to be able to run specific migrations I modified the store file by hand. No matter what I did, I always ended up completely empty store data. In the end, revealed that my JSON file was slightly invalid (because of my modifications), e.g.:

{"foo":"bar",}

notice the extra ,

This causes the fact that when I create the store instance, the data parsing fails and store data is empty. As a side effect, I lose all my data in the json file.

I know editing these files by hand is something people should not really do, but just silently ignoring the parsing error and emptying the file is just super bad IMHO.

IMO better to fail the instantiation if the file is not parseable and not to wipe all the data.

[sindresorhus]

The thinking behind the behavior was that if the JSON parsing failed, it must be because it got corrupted by the system, since the config is not meant to be hand-edited. You can change this behavior with the clearinvalidconfig option. You could argue it should be true by default, and you might be right.

But what would you want to happen? Should the app crash on each launch?

[cloverich]

Could add an onInvalid hook with some options to restore or propagate the error. I would personally prefer a hard-error on invalid parsing so I could catch the bug, but understand how that might surprise some users.

Really its an issue w/ the language (unlike Java or Go, etc, where error conditions are declared as part of the interface), so no choice will satisfy 100% of people.

[moreal]

Could add an onInvalid hook with some options to restore or propagate the error. I would personally prefer a hard-error on invalid parsing so I could catch the bug, but understand how that might surprise some users.

Really its an issue w/ the language (unlike Java or Go, etc, where error conditions are declared as part of the interface), so no choice will satisfy 100% of people.

I agree with the above comment, to add some hook to handle JSON parsing error occurred. Though it had been a long time the last update of this issue So I want to discuss this topic. Could you leave your opinions? @sindresorhus.

If it needs more explanations like that how it worked when the JSON error occurred, please leave a comment 🙏

[oscartbeaumont]

@sindresorhus Like previous messages said, could a function be defined to handle these errors. I use electron-store for a hand editable config and I would really like to prevent the app from starting and alert the user on JSON error instead of wiping the entire config. With guidance I could potentially look at adding this in a PR if your not interested in adding it yourself. This is to fix issue oscartbeaumont/ElectronPlayer#61.

[sindresorhus]

I'm willing to have an error hook added if someone submits a good pull request (that means giving your best effort, adding tests, types, docs, etc).

[sindresorhus]

Version 7 made the clearInvalidConfig option false by default: https://github.com/sindresorhus/electron-store/releases/tag/v7.0.0