diff --git a/jsonrpc2/conn.go b/jsonrpc2/conn.go index 882d15b01..f0e527cc1 100644 --- a/jsonrpc2/conn.go +++ b/jsonrpc2/conn.go @@ -144,18 +144,18 @@ func (c *Connection) Call(ctx context.Context, method string, params interface{} id: Int64ID(atomic.AddInt64(&c.seq, 1)), resultBox: make(chan asyncResult, 1), } + // TODO: rewrite this using the new target/prototype stuff + ctx = event.Start(ctx, method, + Method(method), RPCDirection(Outbound), RPCID(fmt.Sprintf("%q", result.id))) + Started.Record(ctx, 1, Method(method)) + result.ctx = ctx // generate a new request identifier call, err := NewCall(result.id, method, params) if err != nil { - //set the result to failed + // set the result to failed result.resultBox <- asyncResult{err: errors.Errorf("marshaling call parameters: %w", err)} return result } - //TODO: rewrite this using the new target/prototype stuff - ctx = event.Start(ctx, method, - Method(method), RPCDirection(Outbound), RPCID(fmt.Sprintf("%q", result.id))) - Started.Record(ctx, 1, Method(method)) - result.ctx = ctx // We have to add ourselves to the pending map before we send, otherwise we // are racing the response. // rchan is buffered in case the response arrives without a listener. @@ -355,7 +355,7 @@ func (c *Connection) manageQueue(ctx context.Context, preempter Preempter, fromR select { case nextReq, ok = <-fromRead: case toDeliver <- q[0]: - //TODO: this causes a lot of shuffling, should we use a growing ring buffer? compaction? + // TODO: this causes a lot of shuffling, should we use a growing ring buffer? compaction? q = q[1:] } } @@ -413,7 +413,7 @@ func (c *Connection) reply(entry *incoming, result interface{}, rerr error) { } if err := c.respond(entry, result, rerr); err != nil { // no way to propagate this error - //TODO: should we do more than just log it? + // TODO: should we do more than just log it? event.Error(entry.baseCtx, "jsonrpc2 message delivery failed", err) } } @@ -441,7 +441,7 @@ func (c *Connection) respond(entry *incoming, result interface{}, rerr error) er err = errors.Errorf("%w: %q notification failed: %v", ErrInternal, entry.request.Method, rerr) rerr = nil case result != nil: - //notification produced a response, which is an error + // notification produced a response, which is an error err = errors.Errorf("%w: %q produced unwanted response", ErrInternal, entry.request.Method) default: // normal notification finish diff --git a/jsonrpc2/jsonrpc2_test.go b/jsonrpc2/jsonrpc2_test.go index 85a49818c..40df6ed4a 100644 --- a/jsonrpc2/jsonrpc2_test.go +++ b/jsonrpc2/jsonrpc2_test.go @@ -10,6 +10,7 @@ import ( "fmt" "path" "reflect" + "strings" "testing" "time" @@ -60,6 +61,7 @@ var callTests = []invoker{ notify{"unblock", "a"}, collect{"a", true, false}, }}, + callErr{"error", func() {}, "marshaling call parameters: json: unsupported type"}, } type binder struct { @@ -90,6 +92,12 @@ type call struct { expect interface{} } +type callErr struct { + method string + params interface{} + expectErr string +} + type async struct { name string method string @@ -175,6 +183,18 @@ func (test call) Invoke(t *testing.T, ctx context.Context, h *handler) { verifyResults(t, test.method, results, test.expect) } +func (test callErr) Name() string { return test.method } +func (test callErr) Invoke(t *testing.T, ctx context.Context, h *handler) { + var results interface{} + if err := h.conn.Call(ctx, test.method, test.params).Await(ctx, &results); err != nil { + if serr := err.Error(); !strings.Contains(serr, test.expectErr) { + t.Fatalf("%v:Call failed but with unexpected error: %q does not contain %q", test.method, serr, test.expectErr) + } + return + } + t.Fatalf("%v:Call succeeded (%v) but should have failed with error containing %q", test.method, results, test.expectErr) +} + func (test echo) Invoke(t *testing.T, ctx context.Context, h *handler) { results := newResults(test.expect) if err := h.conn.Call(ctx, "echo", []interface{}{test.method, test.params}).Await(ctx, results); err != nil {