googleapis/google-cloudevents-dotnet

Validate Schema: More detailed Error Output

grant opened this issue · 6 comments

grant commented

I'm trying to add JSON examples for our CloudEvent payloads and I'm not seeing a detailed enough error message for me to triage the issue.

Here's the trace from GitHub Actions:

Testing google/events/cloud/cloudbuild/v1/BuildEventData-complex.json
Google.Protobuf.InvalidProtocolBufferException: Expected an object
   at Google.Protobuf.JsonParser.Merge(IMessage message, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.MergeField(IMessage message, FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.Merge(IMessage message, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.MergeField(IMessage message, FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.Merge(IMessage message, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.ParseSingleValue(FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.MergeField(IMessage message, FieldDescriptor field, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.Merge(IMessage message, JsonTokenizer tokenizer)
   at Google.Protobuf.JsonParser.Merge(IMessage message, TextReader jsonReader)
   at Google.Protobuf.JsonParser.Parse(TextReader jsonReader, MessageDescriptor descriptor)
   at Google.Protobuf.JsonParser.Parse(String json, MessageDescriptor descriptor)
   at Google.Events.ValidateSchema.Program.ValidateSchemaData(String testRoot, String file) in /home/runner/work/google-cloudevents/google-cloudevents/google-cloudevents-dotnet/src/Google.Events.ValidateSchema/Program.cs:line 85

From what I can tell, the main error is:

Google.Protobuf.InvalidProtocolBufferException: Expected an object

But I'm not sure where we are expecting an object. It would be nice if I had more information and the error message was clear. I don't know .NET that well as well. Can we add better error messages to src/Google.Events.ValidateSchema/Program.cs?

Source:

https://github.com/googleapis/google-cloudevents/runs/2967111869?check_suite_focus=true

grant commented

Same with errors like "Multiple values specified for oneof source". It's not possible to find the reference of the oneof source.

jskeet commented

The error messages are coming from within the main protobuf library. For the first error, it might be feasible to report what message type we're trying to parse when we encounter a value that isn't a JSON object (when we were expecting a JSON object). That wouldn't help identify which field we were trying to parse or where we were within the JSON, however - both of which are significantly more fiddly to add.

For the second error, it looks reasonably clear to me. There's a oneof called source, and you've specified multiple values for it. There's only a single oneof called source within our repo, which is in BuildEventData.Source, so I strongly suspect you're specifying both storageSource and repoSource for a CloudBuild event.

jskeet commented

@grant: Ping for whether you consider this to still be an issue, given my comment from July 2nd.

grant commented

If there's a way we can make the logs more readable, that would be nice. They aren't all from the protobuf library.

Specifically, I think we can give more readable positive assertions and actionable errors when a normal developer is adding a sample JSON object to the google-cloudevents repo.

For example, I didn't know that logging this string meant that there was a positive assertion:

Testing google/events/cloud/scheduler/v1/SchedulerJobData-simple.json
Testing google/events/cloud/firestore/v1/DocumentEventData-simple.json
Testing google/events/cloud/firestore/v1/DocumentEventData-complex.json
Testing google/events/cloud/storage/v1/StorageObjectData-complex.json

Maybe we could have something like:

- google/events/cloud/scheduler/v1/SchedulerJobData-simple.json (PASSED)
- google/events/cloud/firestore/v1/DocumentEventData-simple.json (PASSED)
- google/events/cloud/firestore/v1/DocumentEventData-complex.json (PASSED)
- google/events/cloud/storage/v1/StorageObjectData-complex.json (FAILED)

Or like some test runners:

✓ google/events/cloud/scheduler/v1/SchedulerJobData-simple.json
✓ google/events/cloud/firestore/v1/DocumentEventData-simple.json
✓ google/events/cloud/firestore/v1/DocumentEventData-complex.json
✗ google/events/cloud/storage/v1/StorageObjectData-complex.json
...

Having the protobuf stacktrace would be okay I guess, although it's really hard to tell the JSON field I need to modify.

Example code I think would be modified:

google-cloudevents-dotnet/src/Google.Events.ValidateSchema/Program.cs

Line 78 in 09727d6

Console.WriteLine($"Testing {relativePath}");

jskeet commented

Personally I think it was already reasonably clear which file was failing: it's the one with the stack trace under it. Anything without a stack trace is a pass.

Still, I can make that more explicit. This won't change the protobuf stack trace, but I'd be surprised if this often caused an issue. I'd expect us normally to create test data from an actual event. (And if it causes an issue in C#, it should cause an issue in other languages - so if at least one language happens to provide a nicer log, it should be easy enough to address.)

Note that for at least some failures, the protobuf stack trace does say what's wrong - for example, it identified the oneof (source) that was causing the problem in your second comment, and if you include a field that is unknown, that's explicitly stated, e.g. "Google.Protobuf.InvalidProtocolBufferException: Unknown field: oldValuse".

jskeet commented

Closing as we've done as much as we can now.

  • 👍1