shoenig / marathonctl

marathonctl is a CLI tool for Marathon
MIT License
100 stars 26 forks source link

error message for bad configuration is ambiguous #8

Open shoenig opened 9 years ago

shoenig commented 9 years ago

If there is any problem with the specified config file, the generic help message is displayed with no information about what was wrong. We should have detailed error messages so it's not a guessing game.

xavierhardy commented 8 years ago

Yes, this is clearly needed, just saying 400 is not sufficient.

tavisto commented 7 years ago

A simple way to do this would to be to simply pass the marathon output from the body of the response to the error message like so:

diff --git a/app.go b/app.go
index d0da79f..ba31699 100644
--- a/app.go
+++ b/app.go
@@ -237,7 +237,7 @@ func (a AppUpdate) update(id string, body io.ReadCloser) {
    Check(e == nil, "failed to get response", e)
    defer response.Body.Close()
    sc := response.StatusCode
-   Check(sc == 200, "bad status code", sc)
+   Check(sc == 200, "bad status code", sc, a.format.Format(response.Body, a.Humanize))
    fmt.Println(a.format.Format(response.Body, a.Humanize))
 }

This produces a message like so (I have a bad boolean in the json):

bad status code 400 {"message":"Invalid JSON","details":[{"path":"/container/docker/forcePullImage","errors":["error.expected.jsboolean"]}]}

Or wrap it all up in a nice format function etc. Just saying 400 is very frustrating especially when marathon already told you what was wrong.

If you like this idea I will be more than happy to write a patch and submit a PR.

xavierhardy commented 7 years ago

Yes, that should be sufficient.

shoenig commented 7 years ago

@tavisto if you have a PR ready, I will gladly merge it

tavisto commented 7 years ago

PR here: https://github.com/shoenig/marathonctl/pull/39