[bugfix] non grpc style error parse result returns Unknown status, it should be re-parse to find inside status #1981
Conversation
|
[CHATOPS:HELP] ChatOps commands.
|
|
[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/bugfix%2Finternal-grpc%2Fnon-grpc-style-error-parsing/b4392cf584b32838a6e99d606c2e99c07949d60e |
Profile Report
|
kpango
force-pushed
the
bugfix/internal-grpc/non-grpc-style-error-parsing
branch
from
5536ae9
to
5c027d8
Compare
Deploying with
|
| Latest commit: |
f2187b0
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://30e89b7b.vald.pages.dev |
| Branch Preview URL: | https://bugfix-internal-grpc-non-grp.vald.pages.dev |
Codecov ReportPatch coverage:
Additional details and impacted files@@ Coverage Diff @@
## main #1981 +/- ##
==========================================
- Coverage 29.68% 29.60% -0.08%
==========================================
Files 364 365 +1
Lines 34156 34236 +80
==========================================
- Hits 10139 10137 -2
- Misses 23602 23680 +78
- Partials 415 419 +4
... and 4 files with indirect coverage changes Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. ☔ View full report in Codecov by Sentry. |
![sonatype-lift[bot]](https://avatars.githubusercontent.com/in/9843?s=60&v=4){kind=small}
| @@ -114,6 +114,9 @@ var ( | |||
| return Errorf(format, args...) | |||
| } | |||
|
|
|||
| // Join represents a function to generate multiple error when the input errors is not nil. | |||
| Join = errors.Join | |||
There was a problem hiding this comment.
typecheck: Join not declared by package errors
ℹ️ Expand to see all @sonatype-lift commands
You can reply with the following commands. For example, reply with @sonatype-lift ignoreall to leave out all findings.
| Command | Usage |
|---|---|
@sonatype-lift ignore |
Leave out the above finding from this PR |
@sonatype-lift ignoreall |
Leave out all the existing findings from this PR |
@sonatype-lift exclude <file|issue|path|tool> |
Exclude specified file|issue|path|tool from Lift findings by updating your config.toml file |
Note: When talking to LiftBot, you need to refresh the page to see its response.
Click here to add LiftBot to another repo.
Help us improve LIFT! (Sonatype LiftBot external survey)
Was this a good recommendation for you? Answering this survey will not impact your Lift settings.
[ 🙁 Not relevant ] - [ 😕 Won't fix ] - [ 😑 Not critical, will fix ] - [ 🙂 Critical, will fix ] - [ 😊 Critical, fixing now ]
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
| // Is represents a function to check whether err and the target is the same or not. | ||
| func Is(err, target error) bool { | ||
| if target == nil || err == nil { | ||
| return err == target |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
| } | ||
| isComparable := reflect.TypeOf(target).Comparable() | ||
| for { | ||
| if isComparable && (err == target || |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
| }); ok && x.Is(target) { | ||
| return true | ||
| } | ||
| switch x := err.(type) { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
type switch on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
kpango
force-pushed
the
bugfix/internal-grpc/non-grpc-style-error-parsing
branch
2 times, most recently
from
6655f6f
to
c1a0645
Compare
| if err != nil { | ||
| return nil | ||
| } | ||
| switch x := err.(type) { |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
type switch on error will fail on wrapped errors. Use errors.As to check for specific errors (errorlint)
internal/errors/errors.go
Outdated
| return nil | ||
| case 1: | ||
| return errs[0] | ||
| case 2: |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 2, in detected (gomnd)
| // Unwrap represents errors.Unwrap. | ||
| func Unwrap(err error) error { | ||
| if err != nil { | ||
| return nil |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
error is not nil (line 195) but it returns nil (nilerr)
kpango
force-pushed
the
bugfix/internal-grpc/non-grpc-style-error-parsing
branch
3 times, most recently
from
a79c577
to
8f5c0c5
Compare
kpango
force-pushed
the
bugfix/internal-grpc/non-grpc-style-error-parsing
branch
from
8f5c0c5
to
6fbe04b
Compare
|
[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/bugfix%2Finternal-grpc%2Fnon-grpc-style-error-parsing/06ffa17c9e1b66b7210f03556ae6f249ed0a927b |
Profile Report
|
| switch l := len(errs); l { | ||
| case 0: | ||
| return nil | ||
| case 1, 2: |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
mnd: Magic number: 2, in detected (gomnd)
… should be re-parse to find inside status Signed-off-by: kpango <kpango@vdaas.org>
kpango
force-pushed
the
bugfix/internal-grpc/non-grpc-style-error-parsing
branch
from
6fbe04b
to
f2187b0
Compare
|
[FOSSA] The scan result will be available at https://app.fossa.com/projects/custom%2B21465%2Fvald/refs/branch/bugfix%2Finternal-grpc%2Fnon-grpc-style-error-parsing/9676086a43def5b82b73a96999457f36c870aaf1 |
Profile Report
|
| case interface{ Unwrap() error }: | ||
| err = x.Unwrap() | ||
| if err == nil { | ||
| return isComparable && err == target || |
There was a problem hiding this comment.
🚫 [golangci] reported by reviewdog 🐶
comparing with == will fail on wrapped errors. Use errors.Is to check for a specific error (errorlint)
|
Could you please show me the overview your changes ( especially, the approach for resolve ) at the PR description? |
Description:
In the case that gRPC Status Parser returns "Unknown", we should look at the internal Error to determine the status.
During the process of modifying gRPC Status Parser, I needed an Error Join that can save gRPC Error without making it a String, so I added a Join.
Changed part of errors.Wrap to Join for LB GW only.
Related Issue:
Versions:
Checklist:
Special notes for your reviewer: