Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Discord's disable breaking updates script breaks if settings.json is invalid #206106

Closed
rafaelsgirao opened this issue Dec 14, 2022 · 7 comments · Fixed by #225357
Closed

Discord's disable breaking updates script breaks if settings.json is invalid #206106

rafaelsgirao opened this issue Dec 14, 2022 · 7 comments · Fixed by #225357

Comments

@rafaelsgirao
Copy link

Describe the bug

Discord's package has a script which disables breaking updates, by looking and modifying the settings.json file.
This script breaks if said settings.json exists but is empty (fails to load the JSON object)
For some reason all values in this file cleared to me and I encountered this issue.
(tested on NixOS 22.11 but the script hasn't changed in master)

Steps To Reproduce

Steps to reproduce the behavior:

  1. Exit discord
  2. Truncate ~/.config/discord/settings.json
  3. Launch discord
  4. Discord should not launch at all (silently, unless launched from a terminal)

Expected behavior

disable-breaking-updates.py should handle the exception json.decoder.JSONDecodeError which arises from this issue.

Screenshots

N/A

Additional context

Some logs of when this happens:

Traceback (most recent call last):
  File "/nix/store/r1r4a5yjkmkjwl0lp0hn9s9fg6sz9c1j-disable-breaking-updates.py/bin/disable-breaking-updates.py", line 27, in <module>
    settings = json.load(settings_file)
  File "/nix/store/zdba9frlxj2ba8ca095win3nphsiiqhb-python3-3.10.8/lib/python3.10/json/__init__.py", line 293, in load
    return loads(fp.read(),
  File "/nix/store/zdba9frlxj2ba8ca095win3nphsiiqhb-python3-3.10.8/lib/python3.10/json/__init__.py", line 346, in loads
    return _default_decoder.decode(s)
  File "/nix/store/zdba9frlxj2ba8ca095win3nphsiiqhb-python3-3.10.8/lib/python3.10/json/decoder.py", line 337, in decode
    obj, end = self.raw_decode(s, idx=_w(s, 0).end())
  File "/nix/store/zdba9frlxj2ba8ca095win3nphsiiqhb-python3-3.10.8/lib/python3.10/json/decoder.py", line 355, in raw_decode
    raise JSONDecodeError("Expecting value", s, err.value) from None
json.decoder.JSONDecodeError: Expecting value: line 1 column 1 (char 0)

Notify maintainers

@devins2518 @Artturin @Infinidoge

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

nix-shell -p nix-info --run "nix-info -m"
 - system: `"x86_64-linux"`
 - host os: `Linux 5.15.80, NixOS, 22.11 (Raccoon), 22.11.20221130.596a8e8`
 - multi-user?: `yes`
 - sandbox: `yes`
 - version: `nix-env (Nix) 2.11.0`
 - channels(root): `"nixos-22.11"`
 - channels(rg): `""`
 - nixpkgs: `/nix/var/nix/profiles/per-user/root/channels/nixos`

~ took 4s 
@Infinidoge
Copy link
Contributor

Infinidoge commented Dec 15, 2022

Hmm, interesting that Discord doesn't launch at all.
This is a pretty simple fix though,

except JSONDecodeError:
    print("[Nix] settings.json is malformed, letting Discord fix itself")

Or something like that

(Now that I think about it, if the settings.json doesn't exist yet, the script could create it)

@vivlim
Copy link
Contributor

vivlim commented Dec 20, 2022

Tangentally related: I installed discord on a new machine, and the disable breaking updates script didn't appear to have any effect until I created an empty settings.json containing just {}. There were other files in ~/.config/discord, but not settings.json. Creating that would be nice.

Also, the only real indication that it was missing is that when I ran discord from a terminal, the first line of the output was [Nix] settings.json doesn't yet exist, can't disable it yet. This is quite easy to miss because it's at the top of the output, so maybe it would be good to show these errors in a zenity dialog or something similar.

@eadwu
Copy link
Member

eadwu commented Dec 21, 2022

I'm surprised this is even in tree - I don't think we should be changing program behavior - especially when you're calling it discord.

I doubt the majority of the users even knew this was happening outside of those involved in PRs for nixpkgs. This is a matter of convenience like stated in the script that has a trade off causing a degradation in the security of the application - causing users to not update - relying on maintainers to do it.

Please create a separate package for this at the same time when fixing this - call it whatever - just not discord|discord-canary|discord-ptb. Or enclose it in an arg that is disabled by default.

@mayl
Copy link
Contributor

mayl commented Jan 3, 2023

I hit this same issue and adding an ~/.config/discord/settings.json with contents {} got it running for me as well.

@bhipple
Copy link
Contributor

bhipple commented Apr 9, 2023

Thanks for opening an issue and describing the fix. This broke for me too and it took me a while to figure out why ...

@Artturin
Copy link
Member

Artturin commented Apr 9, 2023

#225357

@nixos-discourse
Copy link

This issue has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/reconsider-reusing-upstream-tarballs/42524/33

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants