cloudevents.http.from_http binary incorrect error messages
Expected Behavior
When using cloudevents.http.from_http(headers, body), an event that is missing any of the required fields should result in a cloud_exceptions.MissingRequiredFields exception with a message that indicates which field is missing.
Actual Behavior
When given a Binary Cloud Event that is missing a required field ce-id, ce-source, or ce-type, it will return a MissingRequiredFields error with the incorrect error message Failed to find specversion in HTTP request.
Steps to Reproduce the Problem
from cloudevents.http import from_http
from cloudevents.exceptions import MissingRequiredFields
# Correctly does not result in an error if all required fields are present
def test_from_http():
event = from_http({"ce-specversion": "1.0", "ce-id":"123", "ce-type": "test-type", "ce-source": "test-source"}, "{}")
assert event["id"] == "123"
# Returns an incorrect error message
def test_from_http_missing_id_binary():
try:
event = from_http({"ce-specversion": "1.0", "ce-type": "test-type", "ce-source": "test-source"}, "{}")
assert 1 == 2
except MissingRequiredFields as e:
assert "Failed to find specversion in HTTP request" == str(e)
# Returns the appropriate message
def test_from_http_missing_id_structured():
try:
event = from_http({}, "{\"specversion\": \"1.0\", \"type\": \"test-type\", \"source\": \"test-source\"}")
assert 1 == 2
except MissingRequiredFields as e:
assert "Missing required attributes: {'id'}" == str(e)
The code flow is as follows:
- it checks
is_binary(headers)https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/http_methods.py#L46 - It calls
binary_parser.can_readhttps://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/event_type.py#L6-L16 - it calls
has_binary_headershttps://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/sdk/converters/binary.py#L29-L35 has_binary_headerschecks for the presence of all required fields, which in this test case is false because it is missingce-idhttps://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/sdk/converters/util.py#L4-L10- it then falls through and tries to get the specversion here https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/http_methods.py#L57
- specversion is never set and the error gets thrown here https://github.com/cloudevents/sdk-python/blob/b83bfc58eb851f9b91a96f4665754d9bb82cd74e/cloudevents/http/http_methods.py#L64-L67
I think that the solution might be as simple as changing all of the ands to ors in the has_binary_headers method in step 4.
Specifications
- Platform: Mac OS
- Python Version: 3.9.4
I would also like to see this fixed. It would greatly improve the user experience. What I would also consider doing in this step is adding an attribute to the MissingRequiredFields exception that specifies which fields are actually missing. This would make it easier to handle the exception by another application.
We are seeing this issue, and debugging was a headache because of it. It looks like the PR went stale with no outcome from a WG meeting -- is this true?