net/rpc/jsonrpc: return error when reply not set #19588

Closed
SuperGod opened this Issue Mar 17, 2017 · 9 comments

Comments

Projects
None yet
4 participants
@SuperGod

Please answer these questions before submitting your issue. Thanks!

What version of Go are you using (go version)?

go version go1.8 windows/amd64

What operating system and processor architecture are you using (go env)?

set GOARCH=amd64
set GOBIN=
set GOEXE=.exe
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GOOS=windows
set GOPATH=D:\go;D:\code\analyzer
set GORACE=
set GOROOT=D:\devtool\go64
set GOTOOLDIR=D:\devtool\go64\pkg\tool\windows_amd64
set GCCGO=gccgo
set CC=gcc
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\msys64\tmp\go-build642962111=/tmp/go-build -gno-record-gcc-switches
set CXX=g++
set CGO_ENABLED=1
set PKG_CONFIG=pkg-config
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2

What did you do?

I use this code to use jsonrpc:

package main

import (
	"fmt"
	"log"
	"net"
	"net/rpc"
	"net/rpc/jsonrpc"
)

type Arith int

type Result int

func (t *Arith) Multiply(args int, result *Result) error {
	return nil
}

func (t *Arith) DoMap(args int, result *map[string]string) error {
	*result = map[string]string{}
	return nil
}

func (t *Arith) DoMap2(args int, result *map[string]string) error {
	return nil
}

func main() {
	go runServer()
	client, err := jsonrpc.Dial("tcp", fmt.Sprintf("127.0.0.1:9001"))
	if err != nil {
		log.Fatal(err.Error())
	}

	var reply Result
	err = client.Call("Arith.Multiply", 1, &reply)
	if err != nil {
		log.Fatal(err.Error())
	}
	fmt.Println("call Multiply success")
	var ret map[string]string
	err = client.Call("Arith.DoMap", 1, &ret)
	if err != nil {
		log.Fatal(err.Error())
	}
	fmt.Println("call DoMap success")
	err = client.Call("Arith.DoMap2", 1, &ret)
	if err != nil {
		log.Fatal(err.Error())
	}
	fmt.Println("call DoMap2 success")
}

func runServer() {
	arith := new(Arith)
	rpc.Register(arith)
	tcpAddr, err := net.ResolveTCPAddr("tcp", "127.0.0.1:9001")
	if err != nil {
		fmt.Println("ResolveTCPAddr error:", tcpAddr, err.Error())
		return
	}

	listener, err := net.ListenTCP("tcp", tcpAddr)
	if err != nil {
		fmt.Println("listen tcp error:", tcpAddr, err.Error())
		return
	}

	for {
		conn, err := listener.Accept()
		if err != nil {
			continue
		}
		jsonrpc.ServeConn(conn)
	}
}

What did you expect to see?

I think "DoMap2" should return the same of "DoMap"

What did you see instead?

I use "go run client.go" to test the program,but the output is:

call Multiply success
call DoMap success
2017/03/17 18:15:03 invalid error <nil>
exit status 1

It seems “DoMap2” return an error, but "DoMap" and "Multiply" return no error

@bradfitz bradfitz changed the title from jsonrpc return error when reply not set to net/rpc/jsonrpc: return error when reply not set Mar 17, 2017

@bradfitz bradfitz added this to the Go1.9 milestone Mar 17, 2017

@fraenkel

This comment has been minimized.

Show comment
Hide comment
@fraenkel

fraenkel Mar 17, 2017

Contributor

This is correct. JSON-RPC says the result must be non-null on success.
http://www.jsonrpc.org/specification#response_object
DoMap2 doesn't set the result hence a nil flows which is treated as an error.

Contributor

fraenkel commented Mar 17, 2017

This is correct. JSON-RPC says the result must be non-null on success.
http://www.jsonrpc.org/specification#response_object
DoMap2 doesn't set the result hence a nil flows which is treated as an error.

@SuperGod

This comment has been minimized.

Show comment
Hide comment
@SuperGod

SuperGod Mar 18, 2017

But why "Multiply" return no error? I did not set the result of "result *Result"

But why "Multiply" return no error? I did not set the result of "result *Result"

@fraenkel

This comment has been minimized.

Show comment
Hide comment
@fraenkel

fraenkel Mar 18, 2017

Contributor

Because result is not null. An *int is created with a value of 0.

Contributor

fraenkel commented Mar 18, 2017

Because result is not null. An *int is created with a value of 0.

@SuperGod

This comment has been minimized.

Show comment
Hide comment
@SuperGod

SuperGod Mar 18, 2017

OK,thanks, I understand it.

But I think it can easily be confused .

OK,thanks, I understand it.

But I think it can easily be confused .

@fraenkel

This comment has been minimized.

Show comment
Hide comment
@fraenkel

fraenkel Mar 18, 2017

Contributor

The part that might be considered a bug is that the map is not created. I would also wonder if slices have a similar issue given how the return value is created.

Contributor

fraenkel commented Mar 18, 2017

The part that might be considered a bug is that the map is not created. I would also wonder if slices have a similar issue given how the return value is created.

@bradfitz

This comment has been minimized.

Show comment
Hide comment
@bradfitz

bradfitz Mar 23, 2017

Member

For consistency with the *int result case, we should probably make nil maps be valid and treat them like empty maps. Everything else in Go works like that anyway.

I'd accept a patch.

Member

bradfitz commented Mar 23, 2017

For consistency with the *int result case, we should probably make nil maps be valid and treat them like empty maps. Everything else in Go works like that anyway.

I'd accept a patch.

@bradfitz bradfitz modified the milestones: Go1.9Maybe, Go1.9 Mar 23, 2017

@SuperGod

This comment has been minimized.

Show comment
Hide comment
@SuperGod

SuperGod Mar 23, 2017

I test slices,it have the same issue.

I test slices,it have the same issue.

@fraenkel

This comment has been minimized.

Show comment
Hide comment
@gopherbot

This comment has been minimized.

Show comment
Hide comment

CL https://golang.org/cl/38474 mentions this issue.

@gopherbot gopherbot closed this in 0ca8701 Mar 23, 2017

@golang golang locked and limited conversation to collaborators Mar 23, 2018

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.