Consider allowing custom messages with ErrorCodeInvalidParams.

[jmalloc]

Hi there,

I was wondering if you'd consider either removing or making optional this behavior:

jsonrpc2/methods.go

Line 117 in 314a45a

methodErr.Message = ErrorMessageInvalidParams

Wherein the error message used for an ErrorCodeInvalidParams error is replaced with the default message unconditionally.

This loses useful information when the server returns a more specific error message. Perhaps instead this default could be used only when the existing message is empty?

Thanks!

/cc @CWX-iggy

[AdamSLevy]

Thanks for opening an issue to ask this.

However if you read the JSON-RPC 2.0 spec carefully, you will see that the "message" field is not for custom messages. The message that I am setting on the InvalidParams error is the official message for that officially reserved error type.

https://www.jsonrpc.org/specification#error_object

You should use the "data" field to provide additional information about the issue with the invalid params error.

Of course you could always define your own set of custom errors with your own custom error message.

My intention is for this package to be as strictly conforming to the JSON-RPC 2.0 spec as possible. As such I'm not inclined to allow a custom message with a reserved error type.

[jmalloc]

My intention is for this package to be as strictly conforming to the JSON-RPC 2.0 spec as possible.

I agree with this approach 100%, but for what it's worth there is no language in the spec that mandates any specific value for that field. It only states that:

The message SHOULD be limited to a concise single sentence.

IMO the table simply describes the meaning of each code, and does not state that the string in the message column actually MUST appear in that field. I would further argue that the message field becomes meaningless if it does not convey any additional information than what's already known from the code alone.

I hope I don't appear confrontational - I do believe such a change would still leave the module's behavior well within the specification - however if you definitely do not wish to make the change, I understand. Thanks for your work on this and for getting back to me so quickly :)

[AdamSLevy]

Interesting and fair points.

I would further argue that the message field becomes meaningless if it does not convey any additional information than what's already known from the code alone.

I disagree, I think this is what the "data" field is intended for. I also find the spec ambiguous on certain points, and I have probably adopted an overly strict interpretation of it. I think they are prescribing the message for the reserved error codes and that it is standard. Allowing use of the Message field for InvalidParams leads to misuse of the Message field. Such info should go in the Data field.

That being said, ErrorInvalidParams() already returns the Error populated correctly. If a user truly wishes to override the message I could allow it and only populate the message if the error code is InvalidParams AND the message is empty.

[jmalloc]

I also find the spec ambiguous on certain points

This I can definitely agree on :) At our organisation we have our own specification of an internal JSON-RPC API, and I was definitely forced to make certain decisions in the absence of any real guidance from the JSON-RPC v2 spec!

I think this is what the "data" field is intended for.

We do make use of the data field too, specifically to provide context for business-domain errors that are part of our specification where we can document a specific schema for data along with a specific code. Faced with an unknown JSON-RPC client implementation, there is no real way to convey where that additional information is located in the case of such a low level error like invalid parameters, and hence it may not even be logged, for example.

Anyway, thank you for reopening the issue. I will happily submit a PR implemented as you describe, if you like.

[AdamSLevy]

Please go ahead. It should be sufficient to just add a second condition to this if statement

jsonrpc2/methods.go

Line 115 in 314a45a

if methodErr.Code == ErrorCodeInvalidParams {

&& methodErr.Message == ""

Please break the line up at the && if the line length exceeds 80 chars.