wspb: Unexpected removal of publicly used module broke everything!

[xornet-sl]

refers to #311

Is this a joke? Why wspb - a public package got removed?!
Guys, you exposed this, people use it, it is NOT an internal package. Why you just brake everything in PATCH release??!!
I even can not re-create the functionality of wspb as it uses internal package. Why you did that? Was it really a problem just to update the needed dependencies and let it live?

I was updating websocket from 1.8.7 to 1.8.10 and it broke the whole application. It's not a good approach to introduce any breaking changes in PATCH revisions. Mark it deprecated or fix it. Please, perform removals like that in MAJOR updates.
Now I have to fork your repo and maintain it separately or stick with older version. Or most probably find an alternative.

[nhooyr]

It never belonged in the library and is very easy to recreate. The only reason for internal being imported is for buffer repooling which isn't that necessary but also not much code if you want it. See https://github.com/nhooyr/websocket/blob/docs/internal/bpool/bpool.go

It is unfortunate to remove in a patch release I'll agree. I didn't realize they reimplemented github.com/golang/protobuf in terms of google.golang.org/protobuf so there's really nothing wrong with using it right now but regardless, I wanted to make go.mod and go.sum empty, with zero transitive dependencies and I didn't want to wait for v2. See also #297

Here's a standalone implementation:

// Package wspb provides helpers for reading and writing protobuf messages.
package wspb

import (
	"bytes"
	"context"
	"fmt"
	"sync"

	"github.com/golang/protobuf/proto"
	"nhooyr.io/websocket"
)

// Read reads a protobuf message from c into v.
// It will reuse buffers in between calls to avoid allocations.
func Read(ctx context.Context, c *websocket.Conn, v proto.Message) error {
	return read(ctx, c, v)
}

func read(ctx context.Context, c *websocket.Conn, v proto.Message) (err error) {
	defer errdWrap(&err, "failed to read protobuf message")

	typ, r, err := c.Reader(ctx)
	if err != nil {
		return err
	}

	if typ != websocket.MessageBinary {
		c.Close(websocket.StatusUnsupportedData, "expected binary message")
		return fmt.Errorf("expected binary message for protobuf but got: %v", typ)
	}

	b := bpoolGet()
	defer bpoolPut(b)

	_, err = b.ReadFrom(r)
	if err != nil {
		return err
	}

	err = proto.Unmarshal(b.Bytes(), v)
	if err != nil {
		c.Close(websocket.StatusInvalidFramePayloadData, "failed to unmarshal protobuf")
		return fmt.Errorf("failed to unmarshal protobuf: %w", err)
	}

	return nil
}

// Write writes the protobuf message v to c.
// It will reuse buffers in between calls to avoid allocations.
func Write(ctx context.Context, c *websocket.Conn, v proto.Message) error {
	return write(ctx, c, v)
}

func write(ctx context.Context, c *websocket.Conn, v proto.Message) (err error) {
	defer errdWrap(&err, "failed to write protobuf message")

	b := bpoolGet()
	pb := proto.NewBuffer(b.Bytes())
	defer func() {
		bpoolPut(bytes.NewBuffer(pb.Bytes()))
	}()

	err = pb.Marshal(v)
	if err != nil {
		return fmt.Errorf("failed to marshal protobuf: %w", err)
	}

	return c.Write(ctx, websocket.MessageBinary, pb.Bytes())
}

// errdWrap wraps err with fmt.Errorf if err is non nil.
// Intended for use with defer and a named error return.
// Inspired by https://github.com/golang/go/issues/32676.
func errdWrap(err *error, f string, v ...interface{}) {
	if *err != nil {
		*err = fmt.Errorf(f+": %w", append(v, *err)...)
	}
}

var bpool sync.Pool

// Get returns a buffer from the pool or creates a new one if
// the pool is empty.
func bpoolGet() *bytes.Buffer {
	b := bpool.Get()
	if b == nil {
		return &bytes.Buffer{}
	}
	return b.(*bytes.Buffer)
}

// Put returns a buffer into the pool.
func bpoolPut(b *bytes.Buffer) {
	b.Reset()
	bpool.Put(b)
}

[xornet-sl]

Thanks for the fast reply, appreciate it. I'll try to use your suggested implementation.

It never belonged in the library

How come? It was in the package, it could be imported and it uses library's code. I understand that it was an utility module, but it definitely belonged to the package and was very useful along with websocket itself. Just like, for example wsjon, which hasn't (yet?) been deleted.

[nhooyr]

How come? It was in the package, it could be imported and it uses library's code. I understand that it was an utility module, but it definitely belonged to the package and was very useful along with websocket itself. Just like, for example wsjon, which hasn't (yet?) been deleted.

I agree it was useful. The only reason I decided to remove it is because of the third party dependency. wsjson uses the standard library encoding/json implementation and so doesn't add any bloat to the library. It irked a lot of people that the library didn't have an empty go.sum and go.mod as compared to Gorilla.