Add message and field name to binary serialization errors
This adds the message and field name to errors thrown during binary serialization of scalar fields. It should convert a message like:
Error: invalid uint32: -1
To:
Error: cannot encode field com.whatever.MyMessage.my_field to binary: invalid uint32: -1
I followed the wording of the error thrown here for the phrasing: https://github.com/bufbuild/protobuf-es/blob/main/packages/protobuf/src/to-binary.ts#L74
Fixes #983
Thanks for the PR, Ben. We'll need some insight whether / how this impacts performance and bundle size. I think it would be ideal to use the same mechanism for error messages that toJson uses (but writeField is also used for extensions). We need good test coverage to make sure error messages have the expected content and will not regress.
Running npm run perf in packages/protobuf-test:
Edit: Hmm, looks like there aren't any performance tests for toBinary. Let me add them.
Before:
toBinary perf-payload.bin x 5,528 ops/sec ±4.33% (94 runs sampled)
toBinary tiny example.User x 1,098,031 ops/sec ±0.35% (95 runs sampled)
toBinary normal example.User x 177,436 ops/sec ±1.62% (95 runs sampled)
toBinary scalar values x 275,100 ops/sec ±0.76% (97 runs sampled)
toBinary repeated scalar values x 94,595 ops/sec ±0.64% (98 runs sampled)
toBinary map with scalar keys and values x 65,719 ops/sec ±0.58% (95 runs sampled)
toBinary repeated field with 1000 messages x 2,984 ops/sec ±0.56% (98 runs sampled)
toBinary map field with 1000 messages x 729 ops/sec ±0.71% (97 runs sampled)
After:
toBinary perf-payload.bin x 5,452 ops/sec ±1.57% (97 runs sampled)
toBinary tiny example.User x 1,076,997 ops/sec ±0.32% (96 runs sampled)
toBinary normal example.User x 182,993 ops/sec ±0.56% (98 runs sampled)
toBinary scalar values x 277,949 ops/sec ±0.55% (95 runs sampled)
toBinary repeated scalar values x 90,580 ops/sec ±3.47% (92 runs sampled)
toBinary map with scalar keys and values x 63,867 ops/sec ±0.62% (95 runs sampled)
toBinary repeated field with 1000 messages x 2,956 ops/sec ±0.51% (97 runs sampled)
toBinary map field with 1000 messages x 696 ops/sec ±0.79% (94 runs sampled)
Thanks for the updates, Ben. I'll take a look as soon as possible, but have to take care of some Connect work first.
A gentle nudge on this - it would still be of great benefit to have this information.
Sure, here are those changes.