Enable static check sa4006 #13350
Enable static check sa4006 #13350
Conversation
prestonvanloon
force-pushed
the
sa4006
branch
2 times, most recently
from
31cb10b to
67fa629
Compare
prestonvanloon
force-pushed
the
sa4006
branch
from
67fa629 to
9f53ef9
Compare
| @@ -53,13 +53,17 @@ func (c beaconApiValidatorClient) getBeaconBlock(ctx context.Context, slot primi | |||
| ver = produceBlindedBlockResponseJson.Version | |||
| blinded = true | |||
| decoder = json.NewDecoder(bytes.NewReader(produceBlindedBlockResponseJson.Data)) | |||
| } else if errJson == nil { | |||
| return nil, errors.New("failed to query GET REST endpoint: nil result") | |||
There was a problem hiding this comment.
| return nil, errors.New("failed to query GET REST endpoint: nil result") | |
| return nil, errors.Wrap(err, "failed to query GET REST endpoint") |
prestonvanloon
force-pushed
the
sa4006
branch
from
9f53ef9 to
531579c
Compare
| if ctx.Err() != nil { | ||
| return ctx.Err() | ||
| } |
There was a problem hiding this comment.
Why did you insert the error check here, but replaced ctx with _ elsewhere?
There was a problem hiding this comment.
Good question... I started subjectively adding some context liveness checks but only did it in a few places. I can revert if it doesnt make sense
| if ctx.Err() != nil { | ||
| return ctx.Err() | ||
| } |
There was a problem hiding this comment.
And here you ignore the context coming from StartSpan and do an error check on the original context.
There was a problem hiding this comment.
In practice this is fine because the cancellation won't come from tracing, it will come from the parent context, but in principle we should always use the deepest context possible.
There was a problem hiding this comment.
oh lol i expanded the code pane after seeing your slack message - I too missed the interleaved func def 🤦
| if ctx.Err() != nil { | ||
| return nil, ctx.Err() | ||
| } |
| if ctx.Err() != nil { | ||
| return ctx.Err() | ||
| } |
| if ctx.Err() != nil { | ||
| return nil, ctx.Err() | ||
| } |
What type of PR is this?
Other
What does this PR do? Why is it needed?
Enables SA4006 and fixes violations.
This build will fail until this PR is rebased after #13344, therefore this is a draft until that PRmerges.
Which issues(s) does this PR fix?
Other notes for review
https://staticcheck.dev/docs/checks/#SA4006