search

air-verse / air

☁️ Live reload for Go apps
GNU General Public License v3.0
16.38k stars 772 forks source link

Parse errors in .air.toml cause it to be silently ignored #495

Open brontitall opened 8 months ago

brontitall commented 8 months ago

If no config file is specified on the command line and .air.toml has any parse error, then the file is completely ignored, and no error is reported.

I discovered this with an error I made in in regex escaping, but any parse error behaves the same, and it makes diagnosing the config file problem very hard. I ended up having to make an instrumented build of air, which is how I found this bug.

This is literally the first time I've ever even tried air, so excuse me if this is somehow well-known behaviour.

Steps to reproduce:

  1. mkdir testdir ; cd testdir
  2. air init
  3. Make a change to the .air.toml so we can tell it's being read. e.g. change build.cmd to "echo hello", or color.runner to "blue"
  4. run air to see the config change
  5. add an error to the .air.toml file. e.g., remove a close bracket: sed -i.bak '/exclude_dir/s/]$//' .air.toml
  6. run air again and see that no error is reported for the file parse failure, but the config changes we made above are not applied

Cause

The problem seems to be in the defaultPathConfig function. This function loops over the default file names and early returns if one of them parses successfully. If neither of them parses it returns the default config. It is assuming that the only errors possible in loading a config file are that the file doesn't exist, so the error is not reported and moves on.

I'm just returning to go after a several-year absence, so I'm not yet confident enough to submit a PR, but I think this should probably be handled by examining the err returned by readConfByName and, if it's not a "file not found" then reporting the error and aborting.

In the example above, the error just before the end of the loop is: &errors.errorString{s:"(11, 3): missing comma"}. This isn't perfect, but in combination with something like "Failed to parse config file .air.toml: " it would have saved me a lot of time.

toontimbermont commented 6 months ago

Thanks! This pointer probably saved me hours of searching for the missing quote 😅 +1 for the proposed improvement