Copilot commented on code in PR #3183:
URL: https://github.com/apache/dubbo-go/pull/3183#discussion_r2736436801
##########
protocol/dubbo/hessian2/hessian_response.go:
##########
@@ -41,6 +41,51 @@ type DubboResponse struct {
Attachments map[string]any
}
+// GenericException keeps Java exception class and message.
+type GenericException struct {
+ ExceptionClass string
+ ExceptionMessage string
+}
+
+// Error returns a readable error string.
+func (e GenericException) Error() string {
+ if e.ExceptionClass == "" {
+ return e.ExceptionMessage
+ }
+ if e.ExceptionMessage == "" {
+ return e.ExceptionClass
+ }
+ return "java exception: " + e.ExceptionClass + " - " +
e.ExceptionMessage
+}
Review Comment:
`GenericException.Error()` formats messages as `"java exception: " + class +
" - " + message`, which is inconsistent with existing error formatting in this
repo (`"java exception:%s"` with no extra spaces, e.g.
`protocol/dubbo/impl/codec.go:176`) and also makes legacy-string errors change
their `.Error()` output. Consider aligning the formatting (and potentially
treating the legacy-string / `java.lang.Exception` case specially) to avoid
breaking existing callers/tests that compare error strings.
##########
protocol/dubbo/hessian2/hessian_dubbo.go:
##########
@@ -240,9 +240,15 @@ func (h *HessianCodec) ReadBody(rspObj any) error {
}
rsp, ok := rspObj.(*DubboResponse)
if !ok {
- return perrors.Errorf("java exception:%s",
exception.(string))
+ return perrors.Errorf("java exception:%v", exception)
+ }
+ if g, ok := ToGenericException(exception); ok {
+ rsp.Exception = g
+ } else if e, ok := exception.(error); ok {
+ rsp.Exception = e
+ } else {
+ rsp.Exception = perrors.Errorf("java exception:%v",
exception)
}
Review Comment:
`ReadBody` now prioritizes `ToGenericException(exception)` for
`PackageResponse_Exception`. When `exception` is the legacy string body used
for non-OK response statuses, this changes the resulting `Exception.Error()`
format (e.g., adds class / different prefix formatting), which breaks existing
expectations (see `protocol/dubbo/hessian2/hessian_dubbo_test.go:107-110`).
Consider preserving the previous behavior for this path (e.g., keep
`perrors.Errorf("java exception:%s", s)` for string exceptions / non-OK
statuses), or adjust `GenericException.Error()` to remain backward-compatible
for the legacy-string case.
##########
protocol/dubbo/hessian2/hessian_response.go:
##########
@@ -41,6 +41,51 @@ type DubboResponse struct {
Attachments map[string]any
}
+// GenericException keeps Java exception class and message.
+type GenericException struct {
+ ExceptionClass string
+ ExceptionMessage string
+}
+
+// Error returns a readable error string.
+func (e GenericException) Error() string {
+ if e.ExceptionClass == "" {
+ return e.ExceptionMessage
+ }
+ if e.ExceptionMessage == "" {
+ return e.ExceptionClass
+ }
+ return "java exception: " + e.ExceptionClass + " - " +
e.ExceptionMessage
+}
+
+// ToGenericException converts decoded exception to GenericException when
possible.
+func ToGenericException(expt any) (*GenericException, bool) {
+ switch v := expt.(type) {
+ case *GenericException:
+ return v, true
+ case GenericException:
+ return &v, true
+ case *java_exception.DubboGenericException:
+ return &GenericException{ExceptionClass: v.ExceptionClass,
ExceptionMessage: v.ExceptionMessage}, true
+ case java_exception.DubboGenericException:
+ return &GenericException{ExceptionClass: v.ExceptionClass,
ExceptionMessage: v.ExceptionMessage}, true
+ case java_exception.Throwabler:
+ return &GenericException{ExceptionClass: v.JavaClassName(),
ExceptionMessage: v.Error()}, true
+ case string:
+ return parseLegacyException(v), true
Review Comment:
`ToGenericException` converts any `java_exception.Throwabler` into
`GenericException`, which drops throwable-specific details such as stack traces
and prevents downstream code from type-asserting the error (e.g.
`proxy/proxy.go:209` logs `Throwabler.GetStackTrace()`). Unless the intent is
to always erase the original throwable type, consider not converting
`Throwabler` here (only handle `DubboGenericException` + legacy strings), or
preserving the original throwable as the error cause while exposing
`GenericException` metadata separately.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
