marathonctl icon indicating copy to clipboard operation
marathonctl copied to clipboard

error message for bad configuration is ambiguous

Open shoenig opened this issue 10 years ago • 5 comments

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.

shoenig avatar Apr 08 '15 16:04 shoenig

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

xavierhardy avatar Mar 22 '16 07:03 xavierhardy

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.

tavisto avatar Jul 17 '17 22:07 tavisto

Yes, that should be sufficient.

xavierhardy avatar Jul 18 '17 15:07 xavierhardy

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

shoenig avatar Jul 19 '17 01:07 shoenig

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

tavisto avatar Jul 20 '17 20:07 tavisto