search

superfly / flyctl

Command line tools for fly.io services
https://fly.io
Apache License 2.0
1.43k stars 241 forks source link

flyctl masks graphql errors that include paths #1866

Open tvdfly opened 1 year ago

tvdfly commented 1 year ago

Sometimes we get an error response from a graphql query. flyctl tries to parse that response, including the error fields, so we can handle the error (e.g., show it to the user or take some action). BUT, sometimes flyctl will fail to parse the paths array sometimes because it doesn't match the expected type. Then developers get a confusing error message like Error decoding response: json: cannot unmarshal number into Go struct field GraphQLError.Errors.Path of type string, but don't get to see the actual error.

Here's one example: https://community.fly.io/t/decoding-error-with-flyctl-scale-show/11318

Flyctl should show the original graphql error, even if it encounters errors parsing the graphql error in the response.

redjonzaci commented 1 year ago

@alichay what about this? Hope I am not bothering you!

alichay commented 1 year ago

@alichay what about this? Hope I am not bothering you!

all good, don't worry!

The error that inspired this change stopped occurring, so I never ended up pushing for the change to land.

Between this PR and now, we've also moved most of our API calls from the GraphQL endpoint to the Machines API, which means there isn't a whole ton of value in trying to fix this.

redjonzaci commented 1 year ago

@alichay then maybe we can close this too?