Define, document, and release a stable (v1) version.

[creachadair]

This module is currently versioned for development (v0).

Where practical, I try to avoid breaking API changes. However, when I do need to change the API I've adopted the convention of incrementing the minor version number. For other changes I will generally only increment the patch number. Hence, v0.11.1 to v0.11.2 should be a "safe" (non-breaking) upgrade, whereas v0.12.1 to v0.13.0 includes visible API effects that may require client code to be updated.

This issue was created to track the definition and release of a stable (v1) version. Broadly: I would like to trim down the API as much as possible for such a release.

Summary of Changes

package jrpc2

• Export the fields of the Error type, remove unnecessary methods (b11415d, c677bac, 14261a3, 37bcf3b)
• Remove redundant context value functions (d4361e1)
• Remove the built-in "rpc.cancel" implementation (#49)
• Rework the jrpc2.Network return signature (#51)
• Move command-line tools and examples to a submodule (f3f3c4c)
• Rework the text logger interface to not require a standard logger (bfa778d)
• Remove the "AllowV1" client and server settings (#61)
• Remove the request-check hook (#62)
• Remove the parameter wrapping hooks (#82)

package jhttp

• Simplify the Bridge implementation (#65, #67)
• Add a GET bridge (#68)

package channel

• Remove the separate Sender and Receiver interfaces (65bf9ad)
• Remove the unused WithTrigger wrapper channel (a5d1a82)

package handler

• Remove the NewService constructor (0ee4b78)
• Remove the Varint framing (2534128)
• Add support for positional handler function parameters (#59)
• Remove support for variadic handler functions (4780235)

package server

• Simplify the "service" runner API (4bb7b22)
• Make the loop listener interface easier to implement (31ce9d5)
• Make the loop and accepter accept contexts (#66)

Other

• Remove the jctx package (#82)

[creachadair]

Candidates for removal:

1. handler.NewService. Apart from the convenience of extracting the names, this has no real advantage over explicitly constructing the map. Moreover, it cannot handle unexported methods, and the names it assigns are whatever Go uses, not necessarily what a service definition requires. Better to let the caller be explicit. ✅ 0ee4b782
2. channel.Varint. Convert to example code. ✅ 25341281

[creachadair]

I would like to simplify the server package somewhat, and improve the naming. The terms "service", "simple", and "static" are used confusingly, and I think should be cut down.

My current thinking is to consolidate "simple" and "static" into a single concept: A "static" service implementation is one without a meaningful constructor. Instead of a separate type for single-throw services, the package can have a top-level Run function that does the work.

✅ 4bb7b228

[creachadair]

Possible server simplification: Re-combine the Wait and WaitStatus methods, and give the error reported by Wait a documented concrete type. Now, Wait reports the underlying error; with this change it would always return a status value.

❌ Decided not to do this; conflating these two doesn't save much and the semantics are different enough to be worth keeping separate.

[creachadair]

Possible error simplification: Export most or all of the fields of the *Error type, and drop the methods. Support for marshaling and unmarshling JSON is still necessary, but making the type usable directly is more idiomatic.

✅ b11415dd

[creachadair]

Possible server simplification: Now that the server is exposed to handlers via the context, there is no longer any need for separate top-level PushNotify, PushCallback, or ServerMetrics functions.

To get rid of ServerMetrics would require adding a method to expose it, though.

✅ d4361e1d

[creachadair]

Possible server/client simplification: Remove built-in handling for rpc.cancel notifications, see #49 for discussion.

✅ #49

[creachadair]

Simplify use of the jrpc2.Network helper by having it return the address as well as the network type. This would permit the results to be passed directly to net.Listen, e.g.,

lst, err := net.Listen(jrpc2.Network("localhost:2112"))

✅ #51

[creachadair]

Simplifications to the channel package:

• Remove the exported Sender and Receiver types and consolidate Channel.
• Remove the unused WithTrigger wrapper implementation.

✅ 65bf9ad4 (interface) a5d1a82f (wrapper)

[creachadair]

Simplify the jhttp.Bridge type so that we don't need to virtualize request IDs and doubly-encode request messages.

✅ #52

[creachadair]

Further simplify the Error type by removing the now-redundant UnmarshalData and HasData methods. Since the Data field is now exported, there is no longer any need for a method to access the field.

✅ c677bac2 Remove UnmarshalData ✅ 14261a35 Remove HasData

[rferrazz]

It would be nice to have a common unmarshal interface for jrpc2.Request parameters and jrpc2.Response result. In my use case a very similar message can come from either a notification (that is a jrpc2.Request) or a reply and having an abstraction to handle both with the same function will make my codebase less redundant

[creachadair]

It would be nice to have a common unmarshal interface for jrpc2.Request parameters and jrpc2.Response result. In my use case a very similar message can come from either a notification (that is a jrpc2.Request) or a reply and having an abstraction to handle both with the same function will make my codebase less redundant

Do I understand correctly that you'd like to see something like this?

func (r *Request) UnmarshalPayload(v interface{}) error { /* … */ }  // unmarshals parameters
func (r *Response) UnmarshalPayload(v interface{}) error { /* … */ } // unmarshals result

That is, having the same name for both?

[creachadair]

As a workaround in the meantime, you could write a helper:

func UnmarshalPayload(msg, value interface{}) error {
   switch t := msg.(type) {
   case *jrpc2.Request:
      return t.UnmarshalParams(value)
   case *jrpc2.Response:
      return t.UnmarshalResult(value)
   default:
      return errors.New("unknown message type")
   }
}

[rferrazz]

It would be nice to have a common unmarshal interface for jrpc2.Request parameters and jrpc2.Response result. In my use case a very similar message can come from either a notification (that is a jrpc2.Request) or a reply and having an abstraction to handle both with the same function will make my codebase less redundant

Do I understand correctly that you'd like to see something like this?

func (r *Request) UnmarshalPayload(v interface{}) error { /* … */ }  // unmarshals parameters
func (r *Response) UnmarshalPayload(v interface{}) error { /* … */ } // unmarshals result

That is, having the same name for both?

Not just the name. They need to share an interface like:

type PayloadUnmarshaller interface {
    UnmarshalPayload(v interface{}) error
}

So I can write something like this being message a request or a reply:

func doThings(message PayloadUnmarshaller) error {
    ...
    message.UnmarshalPayload(&stuff)
    ...
}

[creachadair]

Yes, that's what I was asking, thanks.

Note that there is no need for the library to define such an interface—if the two methods have the same signature, you could easily define that interface yourself (as you did in the comment above). As a rule, Go interfaces should be defined by their consumer, not their implementors (except, for example, if you have a factory for different implementations).

This certainly isn't a hard change to make. The method names would be less self-describing, though. Let me think about that.

[creachadair]

❌ I decided not to merge #54, cf. https://github.com/creachadair/jrpc2/pull/54#issuecomment-925068474 for context.

I recommend using the workaround from https://github.com/creachadair/jrpc2/issues/46#issuecomment-919448382 for any cases where the two types need to be used similarly.

[creachadair]

✅ Move command-line tools to a separate module (f3f3c4c3). ✅ server: Make the listener interface easier to implement (31ce9d5f). ✅ Add support for dynamic metric labels (55fbb9bc). ✅ Replace DataErrorf with a WithData method on the Error type (37bcf3bd).

[creachadair]

✅ Support positional arguments in the handler package (#59). ✅ Remove support for variadic functions in the handler package (47802359). ✅ Simplify and generalize the text logger interface (bfa778d1).

[creachadair]

Consider removing the AllowV1 server and client options. The intent of this option was to tolerate calls to/from JSON-RPC 1.0 implementations, but that alone isn't enough for interoperability. For example (cf. #44):

• Server replies still contain the v2 version marker, which (some) v1 clients do not accept.
• The v2 Error object has a stricter structure than v1, and the client only accepts v2.

Since the goal of this library is to implement v2 specifically, I do not think it's worthwhile to add extra plumbing for those cases (e.g., tracking v1 shape through the handler, tolerating arbitrary Error geometry). And in that case, the tolerance settings are not carrying their weight.

✅ #61

[creachadair]

Remove the server-side CheckRequest hook. I added this as a place to hook in auth checks, but the value of having the server handle this (vs. deferring to the Handler) is minimal.

✅ #62

[creachadair]

Remove the parameter-wrapping context plumbing hooks. This was meant as a transparent way to encode context settings, but it was a lot of overhead for relatively little use.

✅ #82

[creachadair]

Replace the custom metrics collector plumbing with the standard expvar package. This will make it easier to export metrics to more conventional collectors like Prometheus.

✅ #89

[creachadair]

Simplify the Handler type to be a plain function instead of an interface.

✅ #90

[creachadair]

Rip out the registration mechanism for error code values. In practice any package that wants to define custom codes has to keep track of them anyway, so that the client can coordinate with the server. The additional hook for cosmetics in the default error string is just not worthwhile.

✅ #91

[creachadair]

Look for usage of the server.Run function and see if we can get rid of it or adapt the use.

✅ #92

[creachadair]

Convert handler.Func into a plain type alias and remove the repetition of the type signature in the Check plumbing.

✅ e1278b5

[creachadair]

I think the package has reached a stable enough API that it's time to cut a v1. The remaining tasks will be:

• [x] Do a thorough audit of the package, type, and function docs (#98).
• [x] Update the README to reflect the stable API and remove the old language about pre-v1 tags (3540e6c).
• [x] Update and simplify the playground example linked from the README.
• [x] Survey existing use to make sure I haven't missed something important.

I haven't decided whether to bother with a separate release branch; I think probably not. If a v2 release becomes desirable I'll clone the code into a submodule, it's not that big a project.

[radeksimko]

I'm not sure if it needs to change the API in any way to account for this - probably not necessarily backwards incompatible way - but unless you're already keeping eye, I'd recommend checking out the proposal for structured logging which was recently accepted: https://github.com/golang/go/issues/56345

[creachadair]

Thanks for flagging that. I don't think the server and client debug logs would benefit much from adding structure, but if an application wants to use structured logs in handlers, it would be easy to plumb through a logger via the NewContext server option and handlers could use it, e.g.,

type slogKey struct{}

func WithLogger(ctx context.Context, s *slog.Logger) context.Context {
   return context.WithValue(ctx, slogKey{}, s)
}

func Logger(ctx context.Context) *slog.Logger {
   if s := ctx.Value(slogKey{}); s != nil {
      return s.(*slog.Logger)
   }
   return slog.Default()
}

opts := &jrpc2.ServerOptions{
  NewContext: func() context.Context {
    return WithLogger(context.Background(), myLogger)
  },
}

func Handle(ctx context.Context, req *jrpc2.Request) (any, error) {
   Logger(ctx).Info("hello from handler", "method", req.Method())
   return "ok", nil
}

or words to that effect.

I'm a little disappointed that this API got approved as-written, but I don't see any obvious compatibility concerns. If we later need to bolt structured logging on to the existing debug facility, it wouldn't be too bad to add another hook to the options.

[creachadair]

v1.0.0 is now tagged and available.