fix(github): handle error before variable assignment#7593
fix(github): handle error before variable assignment#7593olblak merged 3 commits intoupdatecli:mainfrom
Conversation
Signed-off-by: Olblak <[email protected]>
There was a problem hiding this comment.
Pull request overview
This pull request fixes a bug where error handling was performed after attempting to use a potentially nil pointer. The code now correctly checks for errors from client.New() before using the returned clientConfig.
Changes:
- Moved error check before variable assignment to prevent nil pointer dereference
- Added nil check before using clientConfig fields (though this is unnecessary)
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if clientConfig != nil { | ||
| g.client = clientConfig.Client | ||
| g.username = clientConfig.Username | ||
| g.token = clientConfig.TokenSource | ||
| g.URL = clientConfig.URL | ||
| } |
There was a problem hiding this comment.
The nil check for clientConfig is unnecessary. When client.New returns an error (checked at line 321), it always returns nil for the clientConfig, and the function returns early at line 322. This code block is only reached when err is nil, and when client.New succeeds (err == nil), it always returns a non-nil ClientConfig pointer. The nil check should be removed and the fields should be assigned directly.
| if clientConfig != nil { | |
| g.client = clientConfig.Client | |
| g.username = clientConfig.Username | |
| g.token = clientConfig.TokenSource | |
| g.URL = clientConfig.URL | |
| } | |
| g.client = clientConfig.Client | |
| g.username = clientConfig.Username | |
| g.token = clientConfig.TokenSource | |
| g.URL = clientConfig.URL |
olblak
changed the title
olblak
changed the title
Fix #7587
Correctly handle error before assignment variable
Test
To test this pull request, you can run the following commands:
Additional Information
Checklist
Tradeoff
Potential improvement