Show a startup error dialog in case of invalid syntax of the config.json about teams-for-linux HOT 15 CLOSED

I mean, we could make the app crash instead if that is the preferred approach.

from teams-for-linux.

aye @dxps, I leave it open but I consider it lower priority that other parts of the system that are currently just not working due the changes from MS from moving from angular to react. Thanks for reporting.

If you want to work on this, do let us know and I can give you some pointers on where to look at.

from teams-for-linux.

Hi @woernsn , thanks a lot for contributing! I can see the approach you have taken and, in general, it should work, but you will need to not fix that config path or it will only work for you. Thanks again for the PR!

from teams-for-linux.

With 1.4.38, if you don't have a config.json file at all, you see:

$ [WARN] No config file found, using default values

on the console, and then a "Configuration error" dialog box. That seems unexpected? Is one forced to have a config.json nowadays? If so, why doesn't the app create a minimal empty one for you automatically?

from teams-for-linux.

@palves, can you test 1.4.39 again when it is available?
The issue should be fixed now.

from teams-for-linux.

Thanks for the URL. I just tried it (deb on Ubuntu 22.04), and it starts fine now, no dialog box. Thanks again.

from teams-for-linux.

Is it throwing an error in terminal ?

from teams-for-linux.

I haven't open it in the terminal, just use the desktop (Gnome Dock) shortcut.

from teams-for-linux.

Is it throwing an error in terminal ?

Yes, it does.

❯ teams-for-linux
[WARN] No config file found, using default values
A JavaScript error occurred in the main process
Uncaught Exception:
SyntaxError: Expected double-quoted property name in JSON at position 224 (line 6 column 1)
at JSON.parse (<anonymous>)
at Conf._deserialize (/opt/teams-for-linux/resources/app.asar/node_modules/conf/dist/source/index.js:67:43)
at get store (/opt/teams-for-linux/resources/app.asar/node_modules/conf/dist/source/index.js:278:43)
at new Conf (/opt/teams-for-linux/resources/app.asar/node_modules/conf/dist/source/index.js:131:32)
at new ElectronStore (/opt/teams-for-linux/resources/app.asar/node_modules/electron-store/index.js:69:3)
at new AppConfiguration (/opt/teams-for-linux/resources/app.asar/app/appConfiguration/index.js:16:49)
at Object.<anonymous> (/opt/teams-for-linux/resources/app.asar/app/index.js:15:19)
at Module._compile (node:internal/modules/cjs/loader:1391:14)
at Module._extensions..js (node:internal/modules/cjs/loader:1451:10)
at Module.load (node:internal/modules/cjs/loader:1214:32)

from teams-for-linux.

The problem would be that the user won't get a clue of the problem.
I'm just proposing to tell the user about the source of the problem: in this case, the config file having an invalid syntax.

from teams-for-linux.

@IsmaelMartinez Yeah, absolutely, it's a low prio thing. Thanks!

Sure, I may try to contribute, even though nowadays I'm a little bit obsessed and caught in the Rust space.

from teams-for-linux.

aye @dxps, I leave it open but I consider it lower priority that other parts of the system that are currently just not working due the changes from MS from moving from angular to react. Thanks for reporting.

If you want to work on this, do let us know and I can give you some pointers on where to look at.

I tried to implement it but am not really a big electron developer.
Let me know if I'm on a totally wrong track 😉.

from teams-for-linux.

Is one forced to have a config.json nowadays? If so, why doesn't the app create a minimal empty one for you automatically?

Good catch!
I never even thought about the usecase to not have a config.

from teams-for-linux.

@palves, can you test 1.4.39 again when it is available? The issue should be fixed now.

Sure thing. Thanks for fixing!

from teams-for-linux.

It should be ready under https://github.com/IsmaelMartinez/teams-for-linux/releases/tag/v1.4.39 (pre-release as I like to keep the blast radius smaller). Thanks for testing it!

from teams-for-linux.