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

Make config errors more easy to debug #39

Closed
JustSimplyKyle opened this issue Nov 29, 2022 · 5 comments · Fixed by #45
Closed

Make config errors more easy to debug #39

JustSimplyKyle opened this issue Nov 29, 2022 · 5 comments · Fixed by #45
Labels
enhancement New feature or request
Milestone

Comments

@JustSimplyKyle
Copy link

Currently I'm in the middle of writing a config.
image
I'm... trying to debug? but the error msg isn't exactly useful
image
I hope that at least we can add an message to tell the user which line of the config has gone wrong

@JustSimplyKyle
Copy link
Author

Another example:
image

@JakeStanger
Copy link
Owner

Hey, yep noticed this yesterday with my own config when I pushed up the breaking changes. A lot of the problem seems to come from Serde and the way untagged enums work - basically it "guesses" the different config options for multiple monitors/bars and if there's anything invalid inside that block (ie the whole bar config) it nopes out.

I also recently updated Corn which adds direct deserialization support, but that doesn't give you line numbers yet which is...inconvenient.

I've got to do some planning around what I can do to resolve that as I don't think it's particularly trivial. For now, I'd recommend the following:

  • Make sure you're aware of the breaking changes present in each of the commits from last night (sys-modules and its tokens -> snake_case; script path -> cmd; custom exec -> on_click)
  • Use a flat config (ie same config for all bars, don't specify monitors) for debugging.
  • Use JSON (or YAML/TOML) for debugging the config. You can use the corn-cli crate to easily convert Corn to any of those.

@JakeStanger JakeStanger added this to the 0.9.0 milestone Nov 29, 2022
@JakeStanger
Copy link
Owner

JakeStanger commented Nov 29, 2022

Looks like there has been a long-standing PR for this in Serde that so far hasn't gained any response from maintainers. It may be possible to patch this in, but I wouldn't be able to publish a patched version to crates.io

serde-rs/serde#1544

It may also be possible to write a custom deserializer to try and better capture the errors.

@JakeStanger
Copy link
Owner

JakeStanger commented Nov 29, 2022

Actually sorry @JustSimplyKyle it looks like I actually misunderstood the main problem in my first response (hurriedly answered at lunch...). This is a problem with the Corn file not being valid, rather than any config changes in Ironbar. That means if you use corn-cli as above you should get much more detailed error output.

I'll leave this open as there clearly does need to be improvements across the board, but the issue you described is out of scope & needs an issue in Corn instead.

@JakeStanger JakeStanger added the enhancement New feature or request label Dec 12, 2022
@JakeStanger
Copy link
Owner

JakeStanger commented Dec 12, 2022

Right, two big improvements on the way:

  1. I've updated Corn to surface parsing errors in their full detail, which means you now get a full set of info for invalid syntax. JSON/TOML/YAML already did this, although not in as much detail.
2022-12-12T23:00:30.509991Z ERROR ironbar: 
   0: Invalid Corn config
   1: An error while parsing the input file.
   1:  --> 2:22
        |
      2 |     monitors.DP-1 =  }
        |                      ^---
        |
        = expected object, array, boolean, null, string, integer, float, or input
   1: 
  1. I have implemented a custom deserializer for the config which gives you some idea of what's invalid. Until Serde improves its untagged enum error handling, this is as good as we'll get.
2022-12-12T23:08:49.796950Z ERROR ironbar: 
   0: Invalid Corn config
   1: An error occurred when deserializing: 
         0: An invalid config was found. The following errors were encountered:
         1: single-bar (b): unknown variant `dfjdjf`, expected one of `clock`, `mpd`, `tray`, `workspaces`, `sys_info`, `launcher`, `script`, `focused`, `custom`
         2:  multi-bar (c): invalid type: map, expected a sequence

      Location:
         src/config.rs:67

      Note: Both the single-bar (type b / error 1) and multi-bar (type c / error 2) config variants were tried. You can likely ignore whichever of these is not relevant to you.
      Suggestion: Please see https://github.com/JakeStanger/ironbar/wiki/configuration-guide#2-pick-your-use-case for more info on the above

2022-12-12T23:12:45.282254Z ERROR ironbar: 
   0: Invalid Corn config
   1: An error occurred when deserializing: Expected array, found 'String("dkfdfkd")'

As for the last example, I've no idea since this is coming from the bar itself rather than loading the config. Hopefully the refactoring I merged in yesterday I got it.

Hoping to merge in the above in the next couple of days.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants