feat: handle CloudEvent and Message responses from function invocation #68
Conversation
We already **kind of** handled them, but these changes make it more intuitive for the user. For example, the function author can respond with `new CloudEvent(...)` or `HTTP.binary(event)` and the framework should handle the response correctly. Signed-off-by: Lance Ball <[email protected]>
lance
force-pushed
the
handle-cloud-event-returns
branch
from
81d47c1 to
ff02d8a
Compare
| @@ -7,10 +8,10 @@ module.exports = function invoker(func) { | |||
| code: 200, | |||
| response: undefined, | |||
| headers: { | |||
| 'Content-Type': 'application/json; charset=utf8', | |||
| 'Access-Control-Allow-Methods': | |||
| 'content-type': 'application/json; charset=utf8', | |||
There was a problem hiding this comment.
Why this change, shouldn't it be case insensitive?
There was a problem hiding this comment.
You're right - it should be, and it is here where we merge user headers with event headers https://github.com/boson-project/faas-js-runtime/pull/68/files#diff-89ae878b1a78765add577244271796f156213cdd0ab6722f641ef96926f38674R54.
That change to lowercase is kind of immaterial since we do lowercase all headers later. It was left over from some debugging I was doing when I realized that I was not doing case-insensitive comparisons. But I thought it would be fine to keep. 🤷
| @@ -226,25 +282,6 @@ test('Successfully handles events with no data', t => { | |||
| }); | |||
| }); | |||
|
|
|||
| test('Responds with error code (4xx or 5xx) to malformed cloud events', t => { | |||
There was a problem hiding this comment.
I probably should have mentioned that in the description. I don't think we should fail if we can help it. If we can recover by creating a CE that is correct, in spite of the malformation, then I think we should do that.
There was a problem hiding this comment.
@matejvasek I will elaborate a little more here. The rationale behind this change is rooted in changes that were made in the upstream cloudevents module and which recently landed in v3.2.0. That version bump of the module introduced the HTTP.binary() API method (and others) that I have made use of in this PR.
But a bigger thing that it did was change how the receiver handles malformed events. A summary of the wider community discussion on why receiving a CloudEvent should provide loose validation is in this comment.
Because of this change in the upstream validation behavior, an exception is never thrown on our end when receiving an event. In cloudevents 3.1.x it would have been and was, and was why this test worked. However, in order to maintain the existing behavior in this module, I would need to add a call to event.validate() when receiving them. To me that seems to land more in the user/customer domain. I thought a lot about this while making the upstream changes, and it's been pretty well hashed out in the community. So I think this module should follow the behavior upstream.
Signed-off-by: Lance Ball <[email protected]>
We already kind of handled them, but these changes make it more intuitive
for the user. For example, the function author can respond with
new CloudEvent(...)or
HTTP.binary(event)and the framework should handle the response correctly.This partially fixes https://github.com/boson-project/functions/issues/15
Signed-off-by: Lance Ball [email protected]