Repository navigation
Emit Binary, but receives Structured #301
Description
Describe the Bug
I was playing around with a small example where i Emit a binary cloud event. something like this:
'use strict';
const { Emitter, CloudEvent } = require('cloudevents');
const em = new Emitter({ url: 'http://localhost:3000/cloudeventy' });
const ce = new CloudEvent({
type: 'com.lholmquist.cloudeventy.fun',
source: 'fun-with-cloud-events',
data: 'DATA'
});
em.send(ce)
.then(response => {
console.log(response);
})
.catch(err => {
console.log(err);
});
Then i have a basic express.js application running on port 3000 with the route cloudeventy to receive the event coming in, which looks similar to this:
app.post('/cloudeventy', (req, res) => {
console.log(req.headers);
const ce = Receiver.accept(req.headers, req.body);
res.send('...');
});
However, I get an error saying that "Cannot read property specversion of undefned".
I also console logged out my request headers too.
{
accept: 'application/json, text/plain, */*',
'content-type': 'application/cloudevents+json; charset=utf-8',
'ce-id': '51568118-f04b-4491-9895-a0e1bfa4ac09',
'ce-type': 'com.lholmquist.cloudeventy.fun',
'ce-source': 'fun-with-cloud-events',
'ce-specversion': '1.0',
'ce-time': '2020-08-04T23:39:35.162Z',
'user-agent': 'axios/0.19.2',
'content-length': '4',
host: 'localhost:3000',
connection: 'close'
}
TypeError: Cannot read property 'specversion' of undefined
at getVersion (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/cloudevents/dist/transport/receiver.js:92:79)
at Function.accept (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/cloudevents/dist/transport/receiver.js:45:25)
at /home/lucasholmquist/develop/just-playing/fun-with-cloudevents/index.js:14:23
at Layer.handle [as handle_request] (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/layer.js:95:5)
at next (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/route.js:137:13)
at Route.dispatch (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/route.js:112:3)
at Layer.handle [as handle_request] (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/layer.js:95:5)
at /home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/index.js:281:22
at Function.process_params (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/index.js:335:12)
at next (/home/lucasholmquist/develop/just-playing/fun-with-cloudevents/node_modules/express/lib/router/index.js:275:10)
When i step through the code on the receiving end, it treats it as a structured event, not a binary event, even though that was how it was sent.
There is code to determine the mode, that parses the content-type header. https://github.com/cloudevents/sdk-javascript/blob/master/src/transport/receiver.ts#L66 and if it sees application/cloudevents then it says it is a "structured" event. The problem is that both the structured and binary emitters add the same content-type header.
Binary Emitter Headers: https://github.com/cloudevents/sdk-javascript/blob/master/src/transport/http/binary_emitter.ts#L23
Structured Emitter Headers: https://github.com/cloudevents/sdk-javascript/blob/master/src/transport/http/structured_emitter.ts#L8
It seems that the check for "Binary Headers" should happen first, unless the headers are wrong for the binary version
Extra Context:
These code examples i gave above i think would make a good example, since we don't have an emitter example yet
Activity
grant commentedon Aug 5, 2020
Edit: Well, I don't know how the emitter works, but I would assume we want the user to specify whether to emit binary or structured.
The code above seems fine.
It seems like we don't specify that message mode when emitting?
lholmquist commentedon Aug 5, 2020
The default with no options is binary, which is here: https://github.com/cloudevents/sdk-javascript/blob/master/src/transport/emitter.ts#L48
I think not specifying is ok, as long as we document that it defaults to something.
I guess i'm not familiar enough with the spec to know if the headers we add should be different than what we do currently for binary mode
lance commentedon Aug 5, 2020
Hmm - looks strange
The content type header here indicates a structured event. However, the inclusion of all of the
ce-*headers is what happens for a binary event. So theReceiveris getting confused. What seems wrong here is that theEmitteris sending the wrong content type header. For the binary transport mode this is typicallyapplication/json. In this case, your data is just a string so that would work, but so wouldtext/plain.This appears to be a bug. This line should be:
Very strange that this hasn't been caught in tests....
lance commentedon Aug 5, 2020
Follow up...
In our
Emittertests, we are testing the content type for a structured mode event here. But the binary transport mode tests omit checking the header. That test should probably be augmented to check the header value, and theEmittercode noted above should be changed. That should resolve this issue.lholmquist commentedon Aug 5, 2020
LOL, just wrote that:
And glad this is a bug and i'm not going crazy :)
lholmquist commentedon Aug 5, 2020
Also, is there a place in the spec, that says which headers are suppose to go with which mode?
lholmquist commentedon Aug 5, 2020
assigning to myself to fix if no one minds
lance commentedon Aug 5, 2020
lance commentedon Aug 5, 2020
@lholmquist
I don't think we need to call
Receiver.acceptin theEmittertest. We just need to be validating our expectations. TheReceiveris tested separately.lholmquist commentedon Aug 5, 2020
yup, was just illustrating why we never picked up the issue until now
fix: Binary emitter should add a basic application/json header.
fix: Binary emitter should add a basic application/json header.