Emit Binary, but receives Structured

[lholmquist]

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

@lance @grant thoughts?

[grant]

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]

but I would assume we want the user to specify whether to emit binary or structured.

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.

It seems like we don't specify that message mode when emitting?

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]

Hmm - looks strange

{
  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'
}

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 the Receiver is getting confused. What seems wrong here is that the Emitter is sending the wrong content type header. For the binary transport mode this is typically application/json. In this case, your data is just a string so that would work, but so would text/plain.

This appears to be a bug. This line should be:

  const contentType: Headers = { [CONSTANTS.HEADER_CONTENT_TYPE]: CONSTANTS.DEFAULT_CONTENT_TYPE };

Very strange that this hasn't been caught in tests....

[lance]

Follow up...

In our Emitter tests, 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 the Emitter code noted above should be changed. That should resolve this issue.

[lholmquist]

LOL, just wrote that:

i looked at the emitter tests and for binary, we look for the headers that are suppose to be there, not the ones that aren't suppose to be.

And we also never call Receiver.accpet in those tests

And glad this is a bug and i'm not going crazy :)

[lholmquist]

Also, is there a place in the spec, that says which headers are suppose to go with which mode?

[lholmquist]

assigning to myself to fix if no one minds

[lance]

Also, is there a place in the spec, that says which headers are suppose to go with which mode?

• Binary: https://github.com/cloudevents/spec/blob/v1.0/http-protocol-binding.md#311-http-content-type
• Structured: https://github.com/cloudevents/spec/blob/v1.0/http-protocol-binding.md#321-http-content-type

[lance]

@lholmquist

And we also never call Receiver.accpet in those tests

I don't think we need to call Receiver.accept in the Emitter test. We just need to be validating our expectations. The Receiver is tested separately.

[lholmquist]

yup, was just illustrating why we never picked up the issue until now