googleapis / gapic-generator-typescript

Generate Typescript API client libraries from Protocol Buffers.
Apache License 2.0
72 stars 24 forks source link

createTask requires body to be a buffer #1301

Open ebidel opened 6 years ago

ebidel commented 6 years ago

Environment details

Steps to reproduce

The following code produces an "invalid encoding" error within the SDK:

  const url = 'https://example.com';

   const client = new cloudTasks.CloudTasksClient();
    const response = await client.createTask({
      parent: client.queuePath(PROJECT_ID, LOCATION, QUEUE),
      task: {
        appEngineHttpRequest: {
          httpMethod: 'POST',
          relativeUri: '/audit',
          body: JSON.stringify({url}),
          headers: {'Content-Type': 'application/json'},
        },
      },
    });

server.js:

...
app.use(bodyParser.json());

app.post('/audit', async (req, resp, next) => {
  let url = req.body.url; // undefined
....
});
stijnvn commented 6 years ago

I can confirm this issue.

When you leave out the JSON.stringify() call when creating the body, you don't get an invalid encoding error, but the response body will be undefined. When this has been fixed, will the explicit conversion to string still be necessary?

ebidel commented 6 years ago

Any update on this?

When this has been fixed, will the explicit conversion to string still be necessary?

Not sure about the internals of what actually makes the request, but other APIs like fetcha nd xhr require you to JSON.stringify the body if it's json.

stephencaldwell commented 6 years ago

The documentation is either incorrect or the wrapper code is, but the body property of the task object needs to be a Buffer or a base64 encoded string.

{
   ...
   body: Buffer.from(JSON.stringify(payload)),
   ...
}

or

{
   ...
   body: Buffer.from(JSON.stringify(payload)).toString('base64'),
   ...
}
stephencaldwell commented 6 years ago

JSON.stringify({url}) is valid.

It's a shorthand syntax for creating object properties that have the same name as the variable.

eg:

const x = "abc";
const y = {x};

console.log(y); // produces { x: 'abc' }
console.log(JSON.stringify({x})); //produces '{"x": "abc"}'
nikmartin commented 6 years ago

Also, just FYI I'm encoding the entire JSON body and NOT setting the headers and it's working fine:

In my task creator:

const task = {
      appEngineHttpRequest: {
        httpMethod: 'POST',
        relativeUri: '/tasks/import',
        body: Buffer.from(JSON.stringify({ storeid: storeID, page: page })),
      },
    };

In my task handler:

const paramsObj = JSON.parse(Buffer.from(req.body, 'base64').toString('utf-8'));

I think the docs are wrong about being able to send JSON bodies, I think all payloads have to be base64 encoded strings.

dinigo commented 5 years ago

What you can do is tell the receiver that the information they receive is in JSON so that, for example, express parses it automagically. This is generalized but extracted from working code

const {CloudTasksClient} = require('@google-cloud/tasks');
const client = new CloudTasksClient();

// content of the task
const payload = {
  foo: 'bar',
};

// configuration for the task
const request = {
  parent: client.queuePath('my-project', 'my-location', 'my-batch'),
  task: {
    appEngineHttpRequest: {
      httpMethod: 'POST',
      relativeUri: '/endpoint',
      body: Buffer.from(JSON.stringify(payload)).toString('base64'),
      headers: {
        'Content-Type': 'application/json',
      },
    },
  },
};
return client.createTask(request);

And the handler would be...


app.post('/endpoint', (req, res) => {
  const {foo} = req.body;
  console.log(foo); // "bar"
  res.send('ok');
});
JustinBeckwith commented 5 years ago

@alexander-fenster To give you the TL;DR of the thread, when you create a new task, it expects you to pass a buffer instead of an object or a string. So instead of this:

appEngineHttpRequest: {
  httpMethod: 'POST',
  relativeUri: '/audit',
  body: JSON.stringify({ url }),
  headers: { 'Content-Type': 'application/json' },
},

You have to Buffer.from the object in question:

appEngineHttpRequest: {
  httpMethod: 'POST',
  relativeUri: '/audit',
  body: Buffer.from(JSON.stringify({ url })),
  headers: { 'Content-Type': 'application/json' },
},

I would have never figured this out if not for the create task sample 🤷‍♂️ . Is there a way we could have the generated API here accept both an object and a buffer?

dinigo commented 5 years ago

@JustinBeckwith

I would have never figured this out if not for the create task sample

Same here. There's where I got the idea. I also remembered that PubSub body in cloud functions was a buffer too.

I agree it would be nice to have the data serialized and deserialized by the library itself.

askdhyan commented 5 years ago

What you can do is tell the receiver that the information they receive is in JSON so that, for example, express parses it automagically. This is generalized but extracted from working code

const {CloudTasksClient} = require('@google-cloud/tasks');
const client = new CloudTasksClient();

// content of the task
const payload = {
  foo: 'bar',
};

// configuration for the task
const request = {
  parent: client.queuePath('my-project', 'my-location', 'my-batch'),
  task: {
    appEngineHttpRequest: {
      httpMethod: 'POST',
      relativeUri: '/endpoint',
      body: Buffer.from(JSON.stringify(payload)).toString('base64'),
      headers: {
        'Content-Type': 'application/json',
      },
    },
  },
};
return client.createTask(request);

And the handler would be...

app.post('/endpoint', (req, res) => {
  const {foo} = req.body;
  console.log(foo); // "bar"
  res.send('ok');
});

Really Works! :)

bcoe commented 4 years ago

:wave: in the documentation in samples we already document the Buffer behavior; @alexander-fenster I think what shakes out of this thread is a feature request for a string body to be accepted, along with a Buffer.

averikitsch commented 3 years ago

@alexander-fenster Do you know if this is specific to this library or the API?

flo-sch commented 3 years ago

I ended up here as well quite confused, while using a Cloud Run container as HTTP target.

I found the following unclear:

  1. that the payload passed to createTask() needs to be sent as a buffer (it is indeed shown in the examples, but not obvious whether this is an actual requirement, or just handled this way in the examples)
  2. that, even though you need to "Bufferize" it, you do not need to parse it from the target handler (which I discovered here, thanks to the request headers) (I really thought the target handler would need to handle an application/octet-stream body with base64 encoded payload)

IMO specifying both of those in the documentation would be beneficial :)

summer-ji-eng commented 2 years ago

👋 in the documentation in samples we already document the Buffer behavior; @alexander-fenster I think what shakes out of this thread is a feature request for a string body to be accepted, along with a Buffer.

It is great to have an example code in sample. As a external user, it is so hard to figure out the use a base 64 string. In the sample code we generated, we only initialize the variable in empty object: const task = {} (code). Maybe @bcoe @sofisl. In generator, we have an sample code for reference. It construct a request with field value. It should help (code).

arvin1408 commented 9 months ago

It really should be mentioned somewhere at least in the documentation (https://cloud.google.com/walkthroughs/cloudtasks/cloudtasks-nodejs) that the base64 formatted string Buffer is a requirement. The "serialization error" were generic, not specific to the body element, thus not helpful in pointing to the body formatting problem. After getting stuck, it's only with luck that one lands on this external thread explaining both the issue and solution.