Notifications #6

Closed
dcharbonnier opened this Issue Jan 27, 2014 · 6 comments

Comments

Projects
None yet
2 participants
@dcharbonnier
Collaborator

dcharbonnier commented Jan 27, 2014

http://www.jsonrpc.org/specification#notification

A Notification is a Request object without an "id" member. A Request object that is a Notification signifies the Client's lack of interest in the corresponding Response object, and as such no Response object needs to be returned to the client. The Server MUST NOT reply to a Notification, including those that are within a batch request.

Notifications are not confirmable by definition, since they do not have a Response object to be returned. As such, the Client would not be aware of any errors (like e.g. "Invalid params","Internal error").

@pocesar

This comment has been minimized.

Show comment
Hide comment
@pocesar

pocesar Jan 28, 2014

Owner

it already has support for notifications, or is there an use case where it's not behaving as expected?

Owner

pocesar commented Jan 28, 2014

it already has support for notifications, or is there an use case where it's not behaving as expected?

@dcharbonnier

This comment has been minimized.

Show comment
Hide comment
@dcharbonnier

dcharbonnier Jan 28, 2014

Collaborator

No, it's not.
https://github.com/dcharbonnier/node-jsonrpc2/tree/feature/notifications

  12 passing (49ms)
  1 failing

  1) json-rpc2 Notification request:
     Error: expected '{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request"},"id":null}' to equal ''
Collaborator

dcharbonnier commented Jan 28, 2014

No, it's not.
https://github.com/dcharbonnier/node-jsonrpc2/tree/feature/notifications

  12 passing (49ms)
  1 failing

  1) json-rpc2 Notification request:
     Error: expected '{"jsonrpc":"2.0","error":{"code":-32600,"message":"Invalid Request"},"id":null}' to equal ''
@pocesar

This comment has been minimized.

Show comment
Hide comment
@pocesar

pocesar Jan 28, 2014

Owner

Hmm I really thought it was working (by the way, the current code coverage is AWFUL, less than 35% I guess, need to improve it a lot).

https://github.com/pocesar/node-jsonrpc2/blob/master/src/connection.js#L99

Owner

pocesar commented Jan 28, 2014

Hmm I really thought it was working (by the way, the current code coverage is AWFUL, less than 35% I guess, need to improve it a lot).

https://github.com/pocesar/node-jsonrpc2/blob/master/src/connection.js#L99

@pocesar

This comment has been minimized.

Show comment
Hide comment
@pocesar

pocesar Jan 28, 2014

Owner

Since it's an HTTP, it should return HTTP code 200? what would the body be? an empty {} or an empty string? The headers should be changed to deal with an empty body.

Owner

pocesar commented Jan 28, 2014

Since it's an HTTP, it should return HTTP code 200? what would the body be? an empty {} or an empty string? The headers should be changed to deal with an empty body.

@pocesar

This comment has been minimized.

Show comment
Hide comment
@pocesar

pocesar Jan 28, 2014

Owner

Take a look at the notification branch

Owner

pocesar commented Jan 28, 2014

Take a look at the notification branch

@dcharbonnier

This comment has been minimized.

Show comment
Hide comment
@dcharbonnier

dcharbonnier Jan 28, 2014

Collaborator

I think 200 and I'm sure an empty body. They don't say anything about transport error, but in case of error we MUST NOT send this information.

Collaborator

dcharbonnier commented Jan 28, 2014

I think 200 and I'm sure an empty body. They don't say anything about transport error, but in case of error we MUST NOT send this information.

@pocesar pocesar closed this in 7e2089d Apr 21, 2014

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