jobs: fix improperly wrapped errors

[rafiss]

refs #42510

I'm working on a linter that detects errors that are not wrapped
correctly, and it discovered these.

Release note: None

[cockroach-teamcity]

This change is 

[ajwerner]

Reviewed 5 of 5 files at r1, all commit messages.
Reviewable status: complete! 1 of 0 LGTMs obtained (waiting on @adityamaru)

[rafiss]

tftr!

bors r=ajwerner

[craig]

Build succeeded:

• GitHub CI (Cockroach)

[adityamaru]

@rafiss just a headsup that this seems to have caused #72371 failures. The reason is that https://github.com/cockroachdb/cockroach/pull/72351/files#diff-6b2c8ccac0d3ee5f528d6661fcc9b37a032f7ac928cbfaebc72a05db34e47b4dR179 will now return nil if the err it is wrapping is nil. We do in fact want to return an error if we find a record, even if getRecord itself didn't return an error, because we are waiting for the record to get reconciled.

I'll send out a fix for this, but might be something to keep in mind when running the new linter! I (along with @miretskiy and @stevendanna) also feel that wrapping a nil error should be disallowed. While it saves a few lines of code it doesn't seem like a very desirable side effect? This might be a discussion for another issue.

[rafiss]

thanks for explaining @adityamaru !

I can add a thing to my linter to try to detect nil wrapped errors.

to be fully robust to the problem, a nil-check could be added to the Wrap function itself. maybe @knz has thoughts on that. (it would be backwards incompatible, but perhaps a good idea)

[knz]

a nil-check could be added to the Wrap function itself.

huh, no - it's a feature:

result, err := somefunc()
return result, errors.Wrap(err, ...)

[stevendanna]

I can add a thing to my linter to try to detect nil wrapped errors.

Nice. The nillness linter catches the "definitely nil" cases, but doesn't catch the "maybe nil" cases.

[stevendanna]

huh, no - it's a feature:

I think that the argument is that while it is a feature, the cost isn't worth the benefit. That said, changing it would put us out-of-line with other similar functions in the Go ecosystem.

[knz]

That said, changing it would put us out-of-line with other similar functions in the Go ecosystem.

:that: