Skip to content

CloudEvent Constructor Validation Error Too Generic #364

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
grant opened this issue Nov 10, 2020 · 4 comments · Fixed by #371
Closed

CloudEvent Constructor Validation Error Too Generic #364

grant opened this issue Nov 10, 2020 · 4 comments · Fixed by #371

Comments

@grant
Copy link
Member

grant commented Nov 10, 2020

Describe the Bug

Hi, I'm trying to debug an error from a different user. They're using this SDK.

Their code is like this:

const respondEvent = new CloudEvent({
    type: 'dev.mink.apply.samples.divide',
    source: 'https://github.com/mattmoor/mink-apply-sample/divide',
    time: new Date(),
    dataContentType: 'application/json',
    data: { foo: 'bar' }
  });

However, running this produces the following output:

invalid extension name%                                                                                                                                                         

I think this output could be improved. I don't really know what is wrong and don't know if the user is really trying to create an extension.

  • Ideally, the error would state the field name and exactly what is wrong with the key or value.
  • Ideally, there's be no error (unless there's something really wrong with the data passed in, but it looks fine).

Workaround:

const respondEvent = new CloudEvent({
    type: 'dev.mink.apply.samples.divide',
    source: 'https://github.com/mattmoor/mink-apply-sample/divide',
    time: new Date(),
    dataContentType: 'application/json',
    data: { foo: 'bar' }
  }, false);

Steps to Reproduce

curl -X POST \
-H 'Content-Type: application/json' \
-H 'ce-type: foo' \
-H 'ce-specversion: 1.0' \
-H 'ce-source: blah' \
-H 'ce-id: 1234' \
-d '{"a": 3, "b": 4}' localhost:8080
invalid extension name%              

Thanks!

@grant
Copy link
Member Author

grant commented Nov 10, 2020

@lance
Copy link
Member

lance commented Nov 13, 2020

The error is occurring because you are using dataContentType instead of datacontenttype for the property name and those properties are not the same thing. In the source you point to above, the regex is validating that the extension conforms to the spec (all lowercase and a-z, 0-9). I agree that it would be useful to change that error to something like this however.

        throw new ValidationError(`invalid extension name: ${key}`);

I would not recommend using the workaround (loose validation) if the intent is for this event to propagate to other systems, given that it is not valid.

@grant
Copy link
Member Author

grant commented Nov 13, 2020

Thanks.

A nice error like in the spec would be good:

CloudEvent ValidationError("dataContentType"): CloudEvents attribute names MUST consist of lower-case letters ('a' to 'z') or digits ('0' to '9') from the ASCII character set. Attribute names SHOULD be descriptive and terse and SHOULD NOT exceed 20 characters in length.

https://github.com/cloudevents/spec/blob/master/spec.md#attribute-naming-convention

@grant
Copy link
Member Author

grant commented Nov 13, 2020

Would like to get feedback from event providers as to the strictness. I imagine some provider probably serves an event with a key >20 characters. Hopefully that doesn't break customer apps. I do see your point on the goal of keeping events clean when propagating.

lance added a commit to lance/sdk-javascript that referenced this issue Nov 25, 2020
This commit changes the messages produced when validating extension names and
values, including the offending name or value in the message, and including
text from the CloudEvent specification, or a link to the type system.

Fixes: cloudevents#364

Signed-off-by: Lance Ball <[email protected]>
lance added a commit that referenced this issue Nov 30, 2020
This commit changes the messages produced when validating extension names and
values, including the offending name or value in the message, and including
text from the CloudEvent specification, or a link to the type system.

Fixes: #364

Signed-off-by: Lance Ball <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants