cloudevents / cloudevents/sdk-python
cloudevents.http.from_http binary incorrect error messages
Nobody has claimed this yet.
- Dominant language
- Python
- Stars
- 342
- Forks
- 65
- PR merge metrics
- No merged PRs in 30d
Description
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
Contributor guide
First steps
- Read the whole issue, then the project's contributing guide.
- Comment on the issue to say you are picking it up — it saves two people doing the same work.
- Fork the repository and make your change on a branch.
- Open a pull request that references the issue number.
Research direction
Start in cloudevents/http/http_methods.py and trace the binary parsing path through cloudevents/http/event_type.py, cloudevents/sdk/converters/binary.py, and cloudevents/sdk/converters/util.py. Reproduce the missing-field cases shown in the issue and verify that missing ce-id, ce-source, or ce-type reports the corresponding missing field instead of specversion.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- python
- Domain
- api
- Issue type
- Bug
- Difficulty
- 2/5
- Estimated time
- 1-3 hours
- Activity status
- Stale
- Clarity
- Clearly specified
- Newbie friendliness
- 45/100