bufbuild / protobuf-es

Protocol Buffers for ECMAScript. The only JavaScript Protobuf library that is fully-compliant with Protobuf conformance tests.
Apache License 2.0
1.16k stars 69 forks source link

Better error messages for asserts #419

Open jamesshannon opened 1 year ago

jamesshannon commented 1 year ago

I've just spend a bunch of time because of a not-very-helpful error message generated by the assert library. Specifically:

Error: invalid int 32: string
    at assertInt32 (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/private/assert.js:29:15)
    at BinaryWriter.int32 (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/binary-encoding.js:155:9)
    at writeScalar (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/private/binary-format-common.js:240:46)
    at Object.writeMessage (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/private/binary-format-proto3.js:49:33)
    at _File.toBinary (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/message.js:74:13)
    at writeMessageField (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/private/binary-format-common.js:233:28)
    at Object.writeMessage (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/private/binary-format-proto3.js:56:33)
    at _ListFilesResponse.toBinary (file:///home/james/Projects/ipaas/webui/my-app/node_modules/@bufbuild/protobuf/dist/esm/message.js:74:13)
    at Object.listFiles (/home/james/Projects/ipaas/webserver/connect.ts:149:18)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5)

However, it provides no message about the field or the value. I eventually modified your code in writeScalar to log out the fieldNo, tied this back to the message, and figured out what was going on (see separate FR).

I would recommend that this function also print out the field name and the value that it was trying to convert (maybe truncated if over 20 bytes or something). Or, more likely, something upstream like makeBinaryFormatProto3, which seems to be the last function which knows the field name, captures the validation error and prints that out.

├── @bufbuild/protobuf@1.1.0
├── @bufbuild/protoc-gen-connect-es@0.8.3
├── @bufbuild/protoc-gen-es@1.1.0
timostamm commented 1 year ago

Thanks for raising this. We provide error messages with context for the JSON format, we should consider doing the same for this case.

Nova38 commented 10 months ago

I had a same issue when constructing a new instant of an message from a plain object and was only told that Error: invalid int 32: string