Tyrrrz / DiscordChatExporter

Exports Discord chat logs to a file
MIT License
7.7k stars 703 forks source link

The program crashes when the 'Settings.dat' file is not parsable as JSON #1050

Closed sokripon closed 1 year ago

sokripon commented 1 year ago

Version

2.4.0

Flavor

GUI (Graphical User Interface)

Export format

No response

Details

When the Settings.dat file is not valid JSON, the program will crash when started.

When does this happen? When installing it through scoop, it creates an empty Settings.dat file for usage in a persistent folder See here. This has the issue that when trying to start the program it will just see the unparsable file and crash.

Solutions:

Steps to reproduce

  1. Have an empty Settings.dat
  2. Start DiscordChatExporter.exe
  3. Crash keyviz_o2rPUflHN1
Tyrrrz commented 1 year ago

Thanks for reporting this. Somebody reported a similar issue on another project, but I was dumbfounded where the empty settings file comes from. Your issue explains it.

When does this happen? When installing it through scoop, it creates an empty Settings.dat file for usage in a persistent folder See here.

Do you know why this is necessary? It's not that difficult to implement the workaround that you described, but I'd argue that an empty settings file shouldn't be created in the first place.

sokripon commented 1 year ago

It's not that difficult to implement the workaround that you described, but I'd argue that an empty settings file shouldn't be created in the first place.

I made a pull request that implements the workaround of having a JSON parsable file https://github.com/ScoopInstaller/Extras/pull/11493 instead of having an empty file, this fixes this issue.

Tyrrrz commented 1 year ago

I made a pull request that implements the workaround of having a JSON parsable file ScoopInstaller/Extras#11493 instead of having an empty file, this fixes this issue.

That's great. Does the file need to exist beforehand for persist to work?

sokripon commented 1 year ago

That's great. Does the file need to exist beforehand for persist to work?

As far as I know, yes. The reason for that is as far as I know that scoop would otherwise create a folder with that name and so the workaround in scoop is to create a file beforehand, thats really only an issue when the installed program doesn't comes with the file.

Tyrrrz commented 1 year ago

I see, makes sense. In that case, I think your PR for the package metadata is the best way forward. I want to avoid silently ignoring empty settings files because it might also indicate at an issue within the app itself.

Tyrrrz commented 1 year ago

By the way, the manifest for YoutubeDownloader looks like this:

https://github.com/ScoopInstaller/Extras/blob/d6cde5da9dc5f0d25b9930475a283e7e503f461e/bucket/youtubedownloader.json#L12-L16

And for LightBulb:

https://github.com/ScoopInstaller/Extras/blob/d6cde5da9dc5f0d25b9930475a283e7e503f461e/bucket/lightbulb.json#L11-L16