cloud,ccl: fix improperly wrapped errors#72349
Conversation
|
This change is |
5917552 to
45c80af
Compare
2001621 to
cdede82
Compare
pkg/cloud/httpsink/http_storage.go
Outdated
| err := errors.Errorf("error response from server: %s %q", resp.Status, body) | ||
| if err != nil && resp.StatusCode == 404 { | ||
| err = errors.Wrapf(cloud.ErrFileDoesNotExist, "http storage file does not exist: %s", err.Error()) | ||
| err = errors.WithDetailf( |
There was a problem hiding this comment.
Hm. So here/elsewhere in the cloud errors, I'm wondering if we want to move this to the hint instead of keeping it in the main error text? If my recollection is correct, some/most client libraries only default to the main error text, so if we change this, do we risk having support requests where a user comes back to us with "hey, our backup airflow job failed with 'pq: object does not exist'" without the actual error we got from the request?
There was a problem hiding this comment.
hm, yeah, if the other error's text is considered critical info in order to be able to debug the problem, then it makes more sense to put it in the main text.
cdede82 to
ccd6232
Compare
|
This is ready for another review |
I'm working on a linter that detects errors that are not wrapped correctly, and it discovered these. Release note: None
ccd6232 to
4957e07
Compare
HonoreDB
left a comment
There was a problem hiding this comment.
Changefeed test helper change
Reviewed 1 of 14 files at r1.
Reviewable status:complete! 1 of 0 LGTMs obtained (waiting on @dt and @stevendanna)
|
tftr! gonna go ahead and merge since the CI failure is from a timezone test that has since been fixed bors r=HonoreDB |
|
Build failed (retrying...): |
|
Build succeeded: |
71877: lint: add new errwrap linter r=ajwerner,knz a=rafiss fixes #42510 This linter checks if we don't correctly wrap errors. The `/* nolint:errwrap */` comment can be used to disable the linter inline. See individual commits for mistakes this linter caught. It had already caught a few in #72353, #72352, #72351, #72350, and #72349. Release note: None Co-authored-by: Rafi Shamim <rafi@cockroachlabs.com>
refs #42510
I'm working on a linter that detects errors that are not wrapped
correctly, and it discovered these.
Release note: None