Add a Client class to json_rpc_2.

Add a Client class to json_rpc_2.

R=rnystrom@google.com

Committed: https://code.google.com/p/dart/source/detail?r=41535

[Bob Nystrom, 2014-11-03 18:43:55]

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
File pkg/json_rpc_2/lib/error_code.dart (right):

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
pkg/json_rpc_2/lib/error_code.dart:38: String name(int errorCode) {
Doc comment.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
pkg/json_rpc_2/lib/error_code.dart:45: default: return null;
I can see the value in not throwing an ArgumentError here since this is likely
to be called from error-handling code, but returning null seems sketchy. How
about "unknown error $errorCode"?

Or, if you only intend this to be called from the exception class, maybe move it
there or don't export it from here?

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
File pkg/json_rpc_2/lib/src/client.dart (right):

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:15: class Client {
Doc comment.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:104: /// send a response, it has no return
value.
What about errors? Is it possible to send a notification and receive any errors?

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:143: /// invokes [callback] without creating
another batch.
Clarify that the batch doesn't end until the outermost batch ends.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:157: response.forEach(_handleResponse);
The recursive call means that multiple layers of nesting may be unwrapped:

[[[a], [b], [c]]]

Is that desired and/or correct?

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:163: if (response.containsKey("result")) {
If it doesn't have a result, that means it's a notification and can just get
dropped?

If so, add a little comment that effect here.

[nweiz, 2014-11-04 02:22:29]

Code review changes

[nweiz, 2014-11-04 02:22:46]

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
File pkg/json_rpc_2/lib/error_code.dart (right):

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
pkg/json_rpc_2/lib/error_code.dart:38: String name(int errorCode) {
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> Doc comment.

Done.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
pkg/json_rpc_2/lib/error_code.dart:45: default: return null;
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> I can see the value in not throwing an ArgumentError here since this is likely
> to be called from error-handling code, but returning null seems sketchy. How
> about "unknown error $errorCode"?
> 
> Or, if you only intend this to be called from the exception class, maybe move
it
> there or don't export it from here?

These error codes are the ones listed in the spec, but it's valid for other
codes to be used. I want to allow users to check whether the package knows a
nice name for the error code without locking them into any behavior if it
doesn't.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
File pkg/json_rpc_2/lib/src/client.dart (right):

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:15: class Client {
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> Doc comment.

Done.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:104: /// send a response, it has no return
value.
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> What about errors? Is it possible to send a notification and receive any
errors?

No, if a notification causes errors those errors are swallowed silently by the
server.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:143: /// invokes [callback] without creating
another batch.
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> Clarify that the batch doesn't end until the outermost batch ends.

Done.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:157: response.forEach(_handleResponse);
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> The recursive call means that multiple layers of nesting may be unwrapped:
> 
> [[[a], [b], [c]]]
> 
> Is that desired and/or correct?

No, changed.

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/src/clien...
pkg/json_rpc_2/lib/src/client.dart:163: if (response.containsKey("result")) {
On 2014/11/03 18:43:55, Bob Nystrom wrote:
> If it doesn't have a result, that means it's a notification and can just get
> dropped?
> 
> If so, add a little comment that effect here.

No, that means it's an error so we call completeError below.

[Bob Nystrom, 2014-11-04 20:28:16]

LGTM!

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
File pkg/json_rpc_2/lib/error_code.dart (right):

https://codereview.chromium.org/691053006/diff/1/pkg/json_rpc_2/lib/error_cod...
pkg/json_rpc_2/lib/error_code.dart:45: default: return null;
On 2014/11/04 02:22:45, nweiz wrote:
> On 2014/11/03 18:43:55, Bob Nystrom wrote:
> > I can see the value in not throwing an ArgumentError here since this is
likely
> > to be called from error-handling code, but returning null seems sketchy. How
> > about "unknown error $errorCode"?
> > 
> > Or, if you only intend this to be called from the exception class, maybe
move
> it
> > there or don't export it from here?
> 
> These error codes are the ones listed in the spec, but it's valid for other
> codes to be used. I want to allow users to check whether the package knows a
> nice name for the error code without locking them into any behavior if it
> doesn't.

SGTM.

https://codereview.chromium.org/691053006/diff/40001/pkg/json_rpc_2/lib/error...
File pkg/json_rpc_2/lib/error_code.dart (right):

https://codereview.chromium.org/691053006/diff/40001/pkg/json_rpc_2/lib/error...
pkg/json_rpc_2/lib/error_code.dart:41: /// If [errorCode] isn't specified,
returns null.
"specified" is a bit ambiguous here. The reader could think you meant specified
*by the caller*. How about "defined in the JSON-RPC spec"?

[nweiz, 2014-11-05 23:23:18]

Code review changes

[nweiz, 2014-11-05 23:23:39]

https://codereview.chromium.org/691053006/diff/40001/pkg/json_rpc_2/lib/error...
File pkg/json_rpc_2/lib/error_code.dart (right):

https://codereview.chromium.org/691053006/diff/40001/pkg/json_rpc_2/lib/error...
pkg/json_rpc_2/lib/error_code.dart:41: /// If [errorCode] isn't specified,
returns null.
On 2014/11/04 20:28:16, Bob Nystrom wrote:
> "specified" is a bit ambiguous here. The reader could think you meant
specified
> *by the caller*. How about "defined in the JSON-RPC spec"?

Done.

[nweiz, 2014-11-05 23:37:49]

Committed patchset #4 (id:60001) manually as 41535 (presubmit successful).