Skip to content

Commit

Permalink
fix: return an unknown service/method exception to client correctly u…
Browse files Browse the repository at this point in the history
…nder multi_service server scenario (#1503)
  • Loading branch information
Marina-Sakai authored Aug 22, 2024
1 parent 1256b7d commit c8843e4
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 7 deletions.
13 changes: 13 additions & 0 deletions pkg/generic/json_test/generic_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -556,6 +556,19 @@ func testRegression(t *testing.T) {
svr.Stop()
}

func TestUnknownError(t *testing.T) {
addr := test.GetLocalAddress()
svr := initMockServer(t, new(mockImpl), addr)

cli := initThriftClient(transport.TTHeader, t, addr, "./idl/mock_unknown_method.thrift", nil, nil, false)
resp, err := cli.GenericCall(context.Background(), "UnknownMethod", reqMsg)
test.Assert(t, resp == nil)
test.Assert(t, err != nil)
test.DeepEqual(t, err.Error(), "remote or network error[remote]: unknown service , method UnknownMethod")

svr.Stop()
}

func initThriftMockClient(t *testing.T, tp transport.Protocol, enableDynamicGo bool, address string) genericclient.Client {
var p generic.DescriptorProvider
var err error
Expand Down
49 changes: 49 additions & 0 deletions pkg/generic/json_test/idl/mock_unknown_method.thrift
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
include "base.thrift"
include "self_ref.thrift"
include "extend.thrift"
namespace go kitex.test.server

enum FOO {
A = 1;
}

struct InnerBase {
255: base.Base Base,
}

struct ExampleReq {
1: required string Msg,
2: FOO Foo,
3: InnerBase InnerBase,
4: optional i8 I8,
5: optional i16 I16,
6: optional i32 I32,
7: optional i64 I64,
8: optional double Double,
255: base.Base Base,
}
struct ExampleResp {
1: required string Msg,
2: string required_field,
3: optional i64 num (api.js_conv="true"),
4: optional i8 I8,
5: optional i16 I16,
6: optional i32 I32,
7: optional i64 I64,
8: optional double Double,
255: base.BaseResp BaseResp,
}
exception Exception {
1: i32 code
2: string msg
}

struct A {
1: A self
2: self_ref.A a
}

service ExampleService extends extend.ExtendService {
ExampleResp ExampleMethod(1: ExampleReq req)throws(1: Exception err)
ExampleResp UnknownMethod(1: ExampleReq req)
}
14 changes: 7 additions & 7 deletions pkg/remote/codec/util.go
Original file line number Diff line number Diff line change
Expand Up @@ -45,17 +45,17 @@ func SetOrCheckMethodName(methodName string, message remote.Message) error {
if message.RPCRole() == remote.Client {
return fmt.Errorf("wrong method name, expect=%s, actual=%s", callMethodName, methodName)
}
inkSetter, ok := ink.(rpcinfo.InvocationSetter)
if !ok {
return errors.New("the interface Invocation doesn't implement InvocationSetter")
}
inkSetter.SetMethodName(methodName)
svcInfo, err := message.SpecifyServiceInfo(ink.ServiceName(), methodName)
if err != nil {
return err
}
if ink, ok := ink.(rpcinfo.InvocationSetter); ok {
ink.SetMethodName(methodName)
ink.SetPackageName(svcInfo.GetPackageName())
ink.SetServiceName(svcInfo.ServiceName)
} else {
return errors.New("the interface Invocation doesn't implement InvocationSetter")
}
inkSetter.SetPackageName(svcInfo.GetPackageName())
inkSetter.SetServiceName(svcInfo.ServiceName)

// unknown method doesn't set methodName for RPCInfo.To(), or lead inconsistent with old version
rpcinfo.AsMutableEndpointInfo(ri.To()).SetMethod(methodName)
Expand Down
3 changes: 3 additions & 0 deletions pkg/remote/message.go
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,9 @@ func (m *message) SpecifyServiceInfo(svcName, methodName string) (*serviceinfo.S
if svcInfo == nil {
return nil, NewTransErrorWithMsg(UnknownService, fmt.Sprintf("unknown service %s, method %s", svcName, methodName))
}
if _, ok := svcInfo.Methods[methodName]; !ok {
return nil, NewTransErrorWithMsg(UnknownMethod, fmt.Sprintf("unknown method %s (service %s)", methodName, svcName))
}
m.targetSvcInfo = svcInfo
return svcInfo, nil
}
Expand Down

0 comments on commit c8843e4

Please sign in to comment.