Arwalk / zig-protobuf

a protobuf 3 implementation for zig.
MIT License
228 stars 26 forks source link

Errors with `Unknown field received in ....` #56

Closed james-callahan closed 1 month ago

james-callahan commented 3 months ago

e.g.

Unknown field received in google.protobuf.pb.FieldOptions protobuf.Extracted{ .tag = 8416, .field_number = 1052, .data = protobuf.ExtractedData{ .RawValue = 2 } }

This is happening when trying to parse proto files with grpc options.

Using the grpc-gateway repository example:

protoc --plugin=protoc-gen-zig=protoc-gen-zig --zig_out=proto -I grpc-gateway/  grpc-gateway/examples/internal/proto/examplepb/a_bit_of_everything.proto 
Unknown field received in google.protobuf.pb.FieldOptions protobuf.Extracted{ .tag = 8338, .field_number = 1042, .data = protobuf.ExtractedData{ .Slice = { 42, 16, 120, 45, 99, 111, 114, 114, 101, 108, 97, 116, 105, 111, 110, 45, 105, 100, 50, 43, 85, 110, 105, 113, 117, 101, 32, 101, 118, 101, 110, 116, 32, 105, 100, 101, 110, 116, 105, 102, 105, 101, 114, 32, 102, 111, 114, 32, 115, 101, 114, 118, 101, 114, 32, 114, 101, 113, 117, 101, 115, 116, 115, 74, 38, 34, 50, 52, 51, 56, 97, 99, 51, 99, 45, 51, 55, 101, 98, 45, 52, 57, 48, 50, 45, 97, 100, 101, 102, 45, 101, 100, 49, 54, 98, 52, 52, 51, 49, 48, 51, 48, 34, 138, 1, 69, 94, 91, 48, 45, 57, 65, 45, 70, 93, 123, 56, 125, 45, 91, 48, 45, 57, 65, 45, 70, 93, 123, 52, 125, 45, 52, 91, 48, 45, 57, 65, 45, 70, 93, 123, 51, 125, 45, 91, 56, 57, 65, 66, 93, 91, 48, 45, 57, 65, 45, 70, 93, 123, 51, 125, 45, 91, 48, 45, 57, 65, 45, 70, 93, 123, 49, 50, 125, 36, 162, 2, 4, 117, 117, 105, 100 } } }
Arwalk commented 3 months ago

Hello, thanks for your report.

I'm not too familiar with the details of GRPC, maybe it is encapsulating the message in some other tag that is implicit with the messaging?

I'll look into it.

james-callahan commented 3 months ago

I'm not too familiar with the details of GRPC, maybe it is encapsulating the message in some other tag that is implicit with the messaging?

I think it's a lack of handling of uninterpreted_option. See these docs for some of the options I frequently see: https://grpc-ecosystem.github.io/grpc-gateway/docs/mapping/customizing_openapi_output/#using-proto-options

Arwalk commented 3 months ago

You're right. Sorry I looked too quickly.

This message is not blocking, just an information. It has been changed as a warning log recently so users can choose to ignore it (as it was previously output to STDERR).

You should still be able to use generated files even if you see this.

Is it actually making problems for you? If so, do you think you could provide us with a minimal example with the behaviour you see and the expected behaviour?

Should this option be interpreted and it's a bug on our side, or were you just opening this as you saw the error log?

This library is not ready to handle GRPC (or simply services), and i don't want to plan it until async is back and stable in the zig language. But if this limitation here prevents someone to do GRPC, we might have to improve things a bit.

Thank you for your time in any case.

james-callahan commented 3 months ago

Is it actually making problems for you? If so, do you think you could provide us with a minimal example with the behaviour you see and the expected behaviour?

Yes:

$ zig build gen-proto
gen-proto
mq run protoc failure
error: unable to spawn /home/user/zig-sdk/.zig-cache/zig-protobuf/protoc/23.4/linux/x86_64/bin/protoc: StderrStreamTooLong
Build Summary: 1/3 steps succeeded; 1 failed (disable with --summary none)
gen-proto transitive failure
mq run protoc failure
error: the following build command failed with exit code 1:
/home/user/zig-sdk/.zig-cache/o/fed22298b20ac99edf014d46c6eace6e/build /usr/bin/zig /home/user/zig-sdk /home/user/zig-sdk/.zig-cache /home/user/.cache/zig --seed 0xc3047bd8 -Z57cfb045af73a5c4 gen-proto

i.e. it seems as though there is a maximum length allowed of stderr, and that all the Unknown field messages exceed that limit?

I also note that the strings are not printed in a useful way: the output I get is ~500 lines that look like:

Unknown field received in google.protobuf.pb.MethodOptions protobuf.Extracted{ .tag = 8338, .field_number = 1042, .data = protobuf.ExtractedData{ .Slice = { 10, 9, 85, 115, 101, 114, 32, 65, 117, 116, 104, 18, 18, 80, 101, 114, 102, 111, 114, 109, 32, 69, 109, 97, 105, 108, 32, 65, 117, 116, 104, 26, 29, 65, 117, 116, 104, 101, 110, 116, 105, 99, 97, 116, 101, 32, 97, 32, 117, 115, 101, 114, 32, 118, 105, 97, 32, 69, 109, 97, 105, 108 } } }

Which appears to be mostly (but not entirely) an ascii string.

james-callahan commented 2 months ago

@Arwalk ping? Should the logging be removed all together? Or should it be opt-in somehow? Or at least remove the serialisation of bytes that fills the screen...

Arwalk commented 2 months ago

Hello James, sorry I couldn't find the time yet to fix this. I plan on working on it on tuesday. I'll probably change the log level to something more reasonable and limit the logging itself to not be too long.

Arwalk commented 2 months ago

Hey @james-callahan , 97cae69ffa97991913b6115e96e522c83a8cc821 should work. Can you tell me if everything's ok for you with it?

james-callahan commented 2 months ago

Hey @james-callahan , 97cae69 should work. Can you tell me if everything's ok for you with it?

Still too much: I'm still getting StderrStreamTooLong. The proto file has enough definitions that it exceeds the limits even without the body there:

$ /home/user/turnkey/zig-sdk/.zig-cache/zig-protobuf/protoc/23.4/linux/x86_64/bin/protoc --plugin=protoc-gen-zig=/home/user/turnkey/zig-sdk/.zig-cache/o/e9be554c4325f0312cc7b37559464166/protoc-gen-zig --zig_out=/home/user/turnkey/zig-sdk/src/proto -I../mono/proto/ ../mono/proto/services/coordinator/public/v1/public_api.proto |& wc 
   3116   10906  111137

The output looks like:

debug: Unknown field received in google.protobuf.pb.MethodOptions 8338

debug: Unknown field received in google.protobuf.pb.MethodOptions 578365826

debug: Unknown field received in google.protobuf.pb.MethodOptions 8338

debug: Unknown field received in google.protobuf.pb.MethodOptions 578365826

debug: Unknown field received in google.protobuf.pb.MethodOptions 8338

debug: Unknown field received in google.protobuf.pb.MethodOptions 578365818

debug: Unknown field received in google.protobuf.pb.MethodOptions 578365826

debug: Unknown field received in google.protobuf.pb.FileOptions 8338

over more than 3000 lines.

Arwalk commented 2 months ago

Ok, i see the problem, there are huge number of unmanaged options that end up in the generator. I scoped the logs of the library and put them in warning-only inside the generator executable, which should allow people to debug their protobuf inputs if they need to while not being bothered when generating their files.

Can you check f80cd489cd253b5f85367a8ffe746dfc619f60c3 ?

Arwalk commented 2 months ago

@james-callahan any news? Does #63 fixes it for you?

james-callahan commented 1 month ago

@james-callahan any news? Does #63 fixes it for you?

I think so! I've lost context for now but I think it compiled/worked