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

Log libStorage error from logVolumeLoopError #899

Merged
merged 1 commit into from
Jun 26, 2017

Conversation

codenrhoden
Copy link
Member

When libStorage would return an error like:

{
  "message": "POST https://api.digitalocean.com/v2/volumes: 422 Storage is not supported for this region",
  "status": 500,
  "error": "POST https://api.digitalocean.com/v2/volumes: 422 Storage is not supported for this region"
}

The only thing printed to the REX-Ray CLI output was
"error.status=500", and not the actual error message. Fix that by
asserting type to goof.HTTPError, then extracting the real message.

Fixes codedellemc/libstorage#429

This ends up being the difference between a final output of:

FATA[0002] error creating volume                         error.status=500 volume=test

vs

FATA[0002] error creating volume                         error.status=500 error.msg=POST https://api.digitalocean.com/v2/volumes: 422 Storage is not supported for this region volume=test

I think the new output is much more helpful

@codenrhoden codenrhoden added this to the 2017.06-2 milestone Jun 20, 2017
@codenrhoden
Copy link
Member Author

I admit I don't fully understand the interaction between golf, goof, and logrus. I tried for about 90 minutes to figure out why something like log.WithError(httpErr), whereby httpErr was created with httpErr, ok := err.(goof.HTTPErr) wouldn't automatically add error.msg to the log fields. error.status shows up on its own, but no other fields do.

I tried every combination of IncludeFieldsWith{Error,String,Format}, but still no luck. I could log the fields directly that way, by doing log.Debugf("%+v", httpErr), but log.WithError() would never show them.

I still think this is a vast improvement, and actually closes out the Epic #776.

When libStorage would return an error like:
```
{
  "message": "POST https://api.digitalocean.com/v2/volumes: 422 Storage is not supported for this region",
  "status": 500,
  "error": "POST https://api.digitalocean.com/v2/volumes:
422 Storage is not supported for this region"
}
```

The only thing printed to the REX-Ray CLI output was
"error.status=500", and not the actual error message. Fix that by
asserting type to goof.HTTPError, then extracting the real message.
@codecov-io
Copy link

codecov-io commented Jun 20, 2017

Codecov Report

Merging #899 into master will decrease coverage by <.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff            @@
##           master    #899      +/-   ##
=========================================
- Coverage     8.8%   8.79%   -0.01%     
=========================================
  Files          25      25              
  Lines        2977    2980       +3     
=========================================
  Hits          262     262              
- Misses       2679    2682       +3     
  Partials       36      36
Impacted Files Coverage Δ
cli/cli/cmds_10_volume.go 0.14% <0%> (-0.01%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 162c602...089ed93. Read the comment docs.

@akutz akutz merged commit c3fb1d8 into rexray:master Jun 26, 2017
@codenrhoden codenrhoden deleted the bugfix/print_libstorage_err branch June 26, 2017 18:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants