Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Cloud events incorrectly unmarshalled: first argument is undefined, 2nd argument differs from production #41

Closed
comolongo opened this issue Jun 17, 2019 · 14 comments · Fixed by #272
Labels
bug Something isn't working

Comments

@comolongo
Copy link

comolongo commented Jun 17, 2019

When Pubsub notifications are sent to a locally hosted cloud event function, the first argument is undefined. It should contain the event information. The 2nd argument though present, does not match production signature; rather than containing metadata about the notification, it contains the entire event object, which should be the contents of argument 1 instead.

My dummy cloud events function:

exports.helloEvents = (data, context) => {
  console.log(data);
  console.log(context);
};

Pub/sub notification being sent:

{
  "subscription": "projects/myproj/subscriptions/mysub",
  "message": {
    "data": "...my base 64 encoded data...",
    "messageId": "1",
    "attributes": {}
  }
}

helloEvents log in production:

Argument 1, data, is:

{  
   "subscription":"projects/myproj/subscriptions/mysub",
   "message":{  
      "data":"...my base 64 encoded data...",
      "messageId":"1",
      "attributes":{}
   }
}

Argument 2, context, is:

{  
   "eventId":"12032d95-089a-4663-9be5-8aafd9d4c623",
   "resource":{  
      "service":"pubsub.googleapis.com",
      "name":"projects/myproj/topics/mytopic"
   },
   "eventType":"google.pubsub.topic.publish",
   "timestamp":"2019-06-16T18:39:44.809Z"
}

On local, the results I get is very different.

  • Argument 1: undefined
  • Argument 2:
{  
   "subscription":"projects/myproj/subscriptions/mysub",
   "message":{  
      "data":"...my base 64 encoded data...",
      "messageId":"1",
      "attributes":{}
   }
}
@comolongo
Copy link
Author

comolongo commented Jun 17, 2019

From looking through the code, my problem is fixed if isBinaryCloudEvent() in wrapEventFunction always returned true at line 433.

Currently the push notification from the pubsub emulator is causing the code to enter the else clause that supports legacy events and CloudEvents in structured content mode. I'm not sure if this is an issue with the pubsub emulator, or if the support for legacy events can be removed, otherwise I'd be happy to push a CL for review.

@burtonjc
Copy link

Maybe this needs to be a separate issue, but I am running into something similar with pubsub triggered functions. In GCF, I have a pubsub triggered function that receives these as it's arguments from an event on the pubsub topic:
argument 1

{
    "@type":"type.googleapis.com/google.pubsub.v1.PubsubMessage",
    "attributes": { "transactionId":"e85cecfe-803b-4fbc-aac0-0116b0864008" },
    "data": "<encoded string>"
}

argument2

{
    "eventId":"793126546972804",
    "timestamp":"2019-10-19T01:03:49.414Z",
    "eventType":"google.pubsub.topic.publish",
    "resource": {
        "service": "pubsub.googleapis.com",
        "name": "projects/gcp-nodejs-demo/topics/demo",
        "type":"type.googleapis.com/google.pubsub.v1.PubsubMessage"
    }
}

Locally, I am trying to test this function with this setup:

  1. Run the function locally through functions-framework with --signature-type=event
  2. Run the pubsub emulator from gcloud
  3. Create a push subscription in the emulator with the push endpoint being the local function
  4. Trigger an event on the topic with the push subscription

This results in argument 1 being undefined and argument 2 being:

{
    "subscription": "projects/gcp-nodejs-demo/subscriptions/local-receivePubSub-demo",
    "message": {
        "data": "<encoded string",
        "messageId": "1",
        "attributes": { "transactionId":"72c28439-5c5d-4e59-a032-e8f622785835" }
    }
}

After some debugging, event.data on this line of the invoker is undefined. Seems to me like this is most likely the PubSub emulator not sending the right message format to the function, but I can't seem to find anywhere to open an issue with them. Any help would be appreciated!

@stephenhandley
Copy link

stephenhandley commented Nov 19, 2019

I'm seeing a simular issue when following the instructions on forwarding from the PubSub emulator to a function described here.

i'm running the functions emulator as follows

functions-framework --target=dispatch --source=build/dispatch --signature-type=event

this is how i'm forwarding topics

async function forwardTopic (topic) {
  const name = topic.name.split('/').pop();
  const result = await topic.createSubscription(`forward-${name}`, {
    pushEndpoint: 'http://localhost:8080',
  });
  console.log(`Forwarded ${name} to functions emulator`);
  return result;
}

my function target just looks like

export function dispatch (event, context) {
  console.log(arguments);
  console.log('hi got dispatch!', { event, context });
}

the output I get after publishing to one of the forwarded topics is as follows

[1] [Arguments] {
[1]   '0': undefined,
[1]   '1':
[1]    { subscription: '[elided...]',
[1]      message:
[1]       { data: '[elided...]', messageId: '4', attributes: {} } } }
[1] hi got dispatch! { event: undefined,
[1]   context:
[1]    { subscription: '[elided...]',
[1]      message:
[1]       { data: '[elided...]', messageId: '4', attributes: {} } } }

@cif
Copy link

cif commented Jan 10, 2020

There is a mismatch between what's currently in the Google Cloud Functions docs for pubsub invocations and what happens when running functions-framework with --signature-type=event then invoking via HTTP.

If you don't specify a context prop in your request body when invoking the function via HTTP e.g. curl http://localhost:8080 -d '{"context": "foo", "data":"<base64data>"}' -H 'Content-Type: application/json' then it will actually delete event.data before passing it to the target function.

You can see why here https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/master/src/invoker.ts#L446 . I don't really understand what the author is getting at with the comments in that block but, at least now the first argument will be defined if we supply a context.

However, there is still a problem because GCF will be invoked from pubsub with a data prop on the event. From the docs:

exports.helloPubSub = (pubSubEvent, context) => {
  const name = pubSubEvent.data
    ? Buffer.from(pubSubEvent.data, 'base64').toString()
    : 'World';

  console.log(`Hello, ${name}!`);
};

Unfortunately, instead of passing the event itself the invoker is passing just the data prop. https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/master/src/invoker.ts#L457

Can any of the contributors here comment on reasoning behind working towards the cloud events spec vs the google runtime, especially considering this repo is housed in GCP?

@cif
Copy link

cif commented Jan 10, 2020

Read more into the knative cloud events spec. The go receiver signature cloudevents.Event struct is using a data prop vs passing directly, it does makes sense to have an envelope.

https://knative.dev/docs/eventing/samples/helloworld/helloworld-go/
https://github.com/cloudevents/sdk-go/blob/master/pkg/cloudevents/event.go#L11

Is there any reason not to simply change https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/master/src/invoker.ts#L457 to

const result = fn(event, context) vs const result = fn(data, context)?

UPDATE: Forked the repo and the tests explain a bit about the intentions. I also read the Cloud Functions Node docs more.

After all that, the shortest path to replicating the pubsub signature is just to change your invocation to this: http://localhost:8080 -d '{"context": "foo", "data": { "data":"<base64data>"} }' -H 'Content-Type: application/json'

UPDATE TO UPDATE:
Adding headers to the request as per the "secret" doc located here https://github.com/cif/functions-framework-nodejs/tree/master/docs#submitting-post-request-to-simulating-a-pubsub-message also does the trick.

-H "Ce-Type: true" \
  -H "Ce-Specversion: true" \
  -H "Ce-Source: true" \
  -H "Ce-Id: true" \

Hope my struggle here helps someone else out, I will say though that making PubSub more prevalent on the docs would have saved me (and perhaps many others that aren't yet familiar with the cloudevent spec) a solid hour 😅

@m0ar
Copy link

m0ar commented Feb 11, 2020

@comolongo @stephenhandley Did you find a solution for this?

@alexsanzdev
Copy link

alexsanzdev commented Feb 14, 2020

Using requestbin.com I saw that the PubSub Emulator behaves just like GCP PubSub (production). I also noticed, by looking at the headers and json data, that GCP PubSub does not send CloudEvents (binary nor structured). And by looking at the code + comments in invoker.ts, I noticed that Cloud Functions Framework only support CloudEvents (binary and structured). So this means unfortunately that Cloud Functions Framework is currently not compatible with GCP PubSub.

It should be fairly easy to fix the code so that it handles GCP PubSub events properly by checking the json data schema/structure.

I provided the requests from both GCP PubSub and PubSub Emulator. I also provided a snippet of the source code where the problem is.

Push request from PubSub Emulator

Accept: text/html, image/gif, image/jpeg, *; q=.2, */*; q=.2
Content-Type: application/json
User-Agent: Java/1.8.0_172
...
{
    "subscription": "projects/my-project/subscriptions/my-sub-1",
    "message": {
        "data": "eyJ0aW1lc3RhbXAiOiIyMDIwLTAyLTE0VDE2OjMxOjMxLjU3NVoiLCJ1c2VySWQiOiJ1c2VyLTAwNyJ9",
        "messageId": "6",
        "attributes": {
            "myCustomId": "ce02c4b1-3536-408f-b6fe-b0db29a5928b"
        }
    }
}

Push request from GCP PubSub (production)

Accept: application/json
Accept-Encoding: gzip,deflate,br
Content-Type: application/json
From: noreply@google.com
User-Agent: APIs-Google; (+https://developers.google.com/webmasters/APIs-Google.html)
...
{
    "message": {
        "attributes": {
            "myCustomId": "9ad2e7c8-3cfa-4872-a658-4e67cfad7a85"
        },
        "data": "eyJ0aW1lc3RhbXAiOiIyMDIwLTAyLTE0VDE4OjQ0OjI4LjE2OFoiLCJ1c2VySWQiOiJ1c2VyLTAwNyJ9",
        "messageId": "801978309740373",
        "message_id": "801978309740373",
        "publishTime": "2020-02-14T18:44:30.209Z",
        "publish_time": "2020-02-14T18:44:30.209Z"
    },
    "subscription": "projects/my-project/subscriptions/my-sub-2"
}

Location of problem in the source code

let data = event.data;
let context = event.context;
if (isBinaryCloudEvent(req)) {
// Support CloudEvents in binary content mode, with data being the whole
// request body and context attributes retrieved from request headers.
data = event;
context = getBinaryCloudEventContext(req);
} else if (context === undefined) {
// Support legacy events and CloudEvents in structured content mode, with
// context properties represented as event top-level properties.
// Context is everything but data.
context = event;
// Clear the property before removing field so the data object
// is not deleted.
context.data = undefined;
delete context.data;
}

Question about the comments: What does "legacy events" mean in this context? Is it legacy version of CloudEvents or is the author referring to GCP PubSub events? Ping @swalkowski

@mykwillis
Copy link

I, too, am struggling with the apparently simple scenario of using functions-framework to host a local background function that acts as a push endpoint for a Cloud PubSub topic. As reported by others here, my function receives an undefined pubSubEvent argument (whereas it receives the message as expected when deployed as a node Cloud Functions).

I have a one line change to invoker.js that makes things work in my particular case (see below), but I'm not sure if it's general purpose or if it breaks other things.

The documentation for Cloud Pub/Sub gives an example HTTP request format of:

{
     "message": {
       "attributes": {
         "key": "value"
       },
       "data": "SGVsbG8gQ2xvdWQgUHViL1N1YiEgSGVyZSBpcyBteSBtZXNzYWdlIQ==",
       "messageId": "136969346945"
     },
     "subscription": "projects/myproject/subscriptions/mysubscription"
   }

This matches what I see, and what @alexsanzdev reported above.

Furthermore, the sample code for a background function is given as:

exports.subscribe = pubsubMessage => {
  // Print out the data from Pub/Sub, to prove that it worked
  console.log(Buffer.from(pubsubMessage.data, 'base64').toString());
};

But when we look at the code in invoker.js, it assumes that the incoming HTTP request will always have a payload at req.body.data. As seen above, the payload for these PubSub messages is actually at req.body.message.

function wrapEventFunction(userFunction) {
    return (req, res) => {
//...
        let data = event.data;  // <-- in the case of PubSub, we want event.message
        let context = event.context;

So to make things work, I can change invoker.js such that it sets data = event.message in the "legacy" (?) code block. Doing this, my function receives what it expects and functions normally.

        else if (context === undefined) {
            // Support legacy events and CloudEvents in structured content mode, with
            // context properties represented as event top-level properties.
            // Context is everything but data.
            context = event;
            // Clear the property before removing field so the data object
            // is not deleted.
            data = event.message;  // <-- This line added
            context.data = undefined;
            delete context.data;

Of course, I don't know what else that would break.

I'm having a hard time understanding how functions-framework could actually be what GCP is using to host Cloud Functions, because the behavior seems to be different than what I see in GCP production. All I can think is that the process of subscribing to a topic with the --trigger-topic option (gcloud functions deploy subscribe --trigger-topic MY_TOPIC --runtime nodejs10) ends up doing something that changes the format of the message sent to the push endpoint?

For completeness, my test script is like:

$ gcloud beta emulators pubsub start --host-port=localhost:9099
$ $(gcloud beta emulators pubsub env-init) # sets PUBSUB_EMULATOR_HOST
$ node_modules/.bin/functions-framework --target=myProcessor --signature-type=event --port=9100
$ node createSubscription.js my-topic-name http://localhost:9100

where createSubscription.js is like:

const { PubSub } = require('@google-cloud/pubsub');
async function main() {
    const [a, b, topicName, endpoint] = process.argv;
    const pubsub = new PubSub();
    const topic = await pubsub.topic(topicName);
    const [topicExists] = await topic.exists();
    if (!topicExists){
        await topic.create();
    }
    const subscriptionName = 'test-subscription');
    const subscription = await topic.createSubscription(
        subscriptionName,
        {
            pushEndpoint: endpoint
        }
    );
    console.log(`Created subscription -> ${endpoint}`);
}

I'm happy to make a PR for the "fix", but I suspect there's more to the story and would appreciate guidance.

@alexsanzdev
Copy link

@mykwillis yeah, I asked my self exact the same question. Is CF framework really used in runtime on GCP production? Because the behavior is different. Or is it depending on whether you have CF framework dependency as production or dev (in package.json).

@mykwillis your fix is nice and simple, and looks more like the missing line than a fix. Maybe it should have been there from the start. According to the comments this if statement branch is for "legacy events and CloudEvents in structured content mode". But what is a legacy event? Is it the event message current production pubsub sends? I yes, then this is the right if branch to put your fix. But if legacy event means something else, then maybe this is not the proper if branch. Did you run the tests?

@mykwillis
Copy link

@alexsanzdev the tests pass, but I don't believe that they are terribly comprehensive. There is no test data that looks anything like the Cloud Pub/Sub request.

Incidentally, there is test data named "GCF legacy event" that looks like this:

      name: 'GCF legacy event',
      body: {
        eventId: 'testEventId',
        timestamp: 'testTimestamp',
        eventType: 'testEventType',
        resource: 'testResource',
        data: { some: 'payload' },
      },
      expectedResource: 'testResource',
    },

So it seems like maybe there should be a test for the PubSub event format, and the associated fix to pull data from correct place, but I'm still stuck scratching my head asking how this works on GCF production.

In any case, this whole section seems to be flat wrong, because following the approach simply doesn't work.

@jrean
Copy link

jrean commented May 28, 2020

To contribute:

I'm on Mac Os Catalina 10.15.4
My local version of Node.js is 10.18.1 as mentioned in the documentation:
https://cloud.google.com/functions/docs/concepts/nodejs-10-runtime

I have Google Pub/Sub Emulator up and running:

docker run --rm -p 8085:8085 --volumes-from gcloud-config gcr.io/google.com/cloudsdktool/cloud-sdk:latest gcloud beta emulators pubsub start --host-port 0.0.0.0:8085

I have Google Cloud Function Framework up and running:

npm run start

> cloud-function@1.0.0 start /Users/xxx/cloud-function
> npx @google-cloud/functions-framework --target=helloPubSub --signature-type=event

npx: installed 52 in 6.245s
Serving function...
Function: helloPubSub
URL: http://localhost:8080/

Pub/Sub has a Project created.
Pub/Sub has a Topic created.
Pub/Sub has a Subscription created with pushConfig sets to http://host.docker.internal:8080

All good! I'm able to publish a Message to the existing Topic and the Subscription pushes to the Google Cloud Function Framework on the helloPubSub method.

According to the documentation and for testing purposes:
The helloPubSub method is as follow:
https://cloud.google.com/functions/docs/calling/pubsub

/**
 * Background Cloud Function to be triggered by Pub/Sub.
 * This function is exported by index.js, and executed when
 * the trigger topic receives a message.
 *
 * @param {object} pubSubEvent The event payload.
 * @param {object} context The event metadata.
 */
exports.helloPubSub = (pubSubEvent, context) => {
  const name = pubSubEvent.data
    ? Buffer.from(pubSubEvent.data, 'base64').toString()
    : 'World';

  console.log(`Hello, ${name}!`);
  return;
};

According to the documentation and for testing purposes:
The publish message request to Pub/Sub emulator is as follow (and works well):
https://cloud.google.com/pubsub/docs/reference/rest/v1/projects.topics/publish

curl --location --request POST 'http://0.0.0.0:8085/v1/projects/xxx/topics/myNewTopic:publish' \
--header 'Content-Type: application/json' \
--data-raw '{
	"messages": [
		{
			"data": "SGVsbG8gSmVhbiBKZWFu",
			"attributes": {
				"name": "john"
			}
		}
	]
}'

Problem n1:
When I publish the message, Pub/Sub emulator receives it and triggers the Google Cloud Function Framework method but pubSubEvent parameter is undefined!
The context parameter contains:

{
  subscription: 'projects/xxx/subscriptions/myNewSubscription',
  message: {
    data: 'SGVsbG8gSmVhbiBKZWFu',
    messageId: '15',
    attributes: { name: 'john' }
  }
}

So at the end, out of the box the code doesn't work.
The only way to make it works is to change it as follow using context :
Which is not the right way, right?

exports.helloPubSub = (pubSubEvent, context) => {
  const name = context.message.data
    ? Buffer.from(context.message.data, 'base64').toString()
    : 'World';

  console.log(`Hello, ${name}!`);
  return;
};

// Prints => Hello, Hello Jean Jean

Problem n2:
Still following documentations, we can simulate a Pub/Sub message sending a POST request to Google Cloud Function Framework as follow:
https://github.com/GoogleCloudPlatform/functions-framework-nodejs/blob/master/docs/events.md

curl --location --request POST 'http://localhost:8080' \
--header 'Ce-Type: true' \
--header 'Ce-Specversion: true' \
--header 'Ce-Source: true' \
--header 'Ce-Id: true' \
--header 'Content-Type: application/json' \
--data-raw '{
  "message": {
    "attributes": {
      "key": "value"
    },
    "data": "SGVsbG8gQ2xvdWQgUHViL1N1YiEgSGVyZSBpcyBteSBtZXNzYWdlIQ==",
    "messageId": "136969346945"
  },
  "subscription": "projects/myproject/subscriptions/mysubscription"
}'

This time pubSubEvent parameter contains:

{
  message: {
    attributes: { key: 'value' },
    data: 'SGVsbG8gQ2xvdWQgUHViL1N1YiEgSGVyZSBpcyBteSBtZXNzYWdlIQ==',
    messageId: '136969346945'
  },
  subscription: 'projects/myproject/subscriptions/mysubscription'
}

And context parameter contains:

{ type: 'true', specversion: 'true', source: 'true', id: 'true' }

Again, the original version of helloPubSub doesn't work as expected and returns Hello World!

On this page:
https://cloud.google.com/functions/docs/calling/pubsub
Search for This snippet shows the subscribe function that is triggered when the message is published to the Pub/Sub topic:
And look at the given code snippet:
This time there is only 1 parameter and not 2 like in the helloPubSub example.

/**
 * Triggered from a message on a Cloud Pub/Sub topic.
 *
 * @param {object} pubsubMessage The Cloud Pub/Sub Message object.
 * @param {string} pubsubMessage.data The "data" property of the Cloud Pub/Sub Message.
 */
exports.subscribe = (pubsubMessage) => {
  // Print out the data from Pub/Sub, to prove that it worked
  console.log(Buffer.from(pubsubMessage.data, 'base64').toString());
};

And it doesn't work. pubsubMessage parameter being undifined.

J.

@grant
Copy link
Contributor

grant commented May 28, 2020

Hey all, thanks for the comments. I know this is an issue. I'm not sure of the exact solution right now.

We're formally supporting CloudEvents in the Functions Framework spec and will be looking at better, more accurate support for CloudEvents (and legacy events) in the frameworks.

I've pinged a colleague to see if we can also improve our docs on cloud.google.com

In any case, this whole section seems to be flat wrong, because following the approach simply doesn't work.

I've removed that section until we have accurate docs.

@l0co
Copy link

l0co commented Jun 3, 2020

If someone (like me) doesn't care for CloudEvents which require to build big JSON-s plus add custom headers to the request to test simple things, and which are additionally not compatible with current GCP PubSub, can use the following format instead of this described in docs.

In GCP if I send the {"a": 2}" event to a cloud function I get in this function (event, context) parameters the following values:

The event is:

{
    "a": 2
}

Context is:

{
    "eventId": "450000223",
    "resource": {
      "service": "pubsub.googleapis.com",
      "name": "projects/xxx/topics/yyy"
    },
    "eventType": "google.pubsub.topic.publish",
    "timestamp": "2020-06-03T13:20:06.789Z"
}

The same can be achieved with the following request to functions-framework endpoint:

{
  "data": {
    "a": 2
  },
  "context": {
    "eventId": "450000223",
    "resource": {
      "service": "pubsub.googleapis.com",
      "name": "projects/xxx/topics/yyy"
    },
    "eventType": "google.pubsub.topic.publish",
    "timestamp": "2020-06-03T13:20:06.789Z"
  }
}

Or even if you don't care about the context:

{
  "data": {
    "a": 2
  }
}

In my opinion this format should be included in docs before CloudEvents format description.

@andykps
Copy link

andykps commented Mar 18, 2021

This still seems to be a problem.

I'm not a JS dev and I'm concerned that I don't really understand what else functions-framework is trying to support but...

If we assume that when the request contains message and subscription attributes it means that it originated from pubsub then this extra else block in wrapEventFunction of invoker.js works for me to match the structure of the request and what I'm testing locally then matches what happens when deployed to GCP.

        else if (event.message && event.subscription) {
            // This is a message from a pubsub subscription
            data = {
                "@type": "type.googleapis.com/google.pubsub.v1.PubsubMessage",
                attributes: event.message.attributes,
                data: event.message.data
            }
            context = {
                eventId: event.message.messageId,
                timestamp: event.message.publishTime,
                eventType: "google.pubsub.topic.publish",
                resource: {
                    service: "pubsub.googleapis.com",
                    name: event.subscription,
                    type: "type.googleapis.com/google.pubsub.v1.PubsubMessage"
                }
            }
        }

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.