actions-on-google / actions-on-google-nodejs

Node.js client library for Actions on Google
https://actions-on-google.github.io/actions-on-google-nodejs
Apache License 2.0
899 stars 193 forks source link

AWS Lambda callback response format is not correct #120

Open igilham opened 6 years ago

igilham commented 6 years ago

I'm using a basic Hello World app to test the Actions SDK v2.0.1 in the Simulator using Bespoken Tools' Lambda proxy. Note that I had to use node-mocks-http to build a fake event object similar to what the SDK's Lambda framework expects from an API Gateway event.

The Lambda callback is incorrect in this scenario, leading to a parse error when the Assistant service receives the response from the fullfillment app.

'use strict';

const {actionssdk} = require('actions-on-google');
const expressMockery = require("node-mocks-http");

const app = actionssdk({debug: true});

// intent handler
app.intent('actions.intent.MAIN', (conv) => {
  conv.close('Hello world!');
});

// When using API Gateway it should be as easy as this, but it doesn't work like that with BST proxy
// exports.handler = app;

exports.handler = async (event, context, callback) => {
    try {
        // mock an API Gateway request
        let request = expressMockery.createRequest({
            body: JSON.stringify(event),
            headers: []
        });

        await app(request, context, callback);
    } catch(err) {
        callback(err);
    }
}

The Actions Console Simulator returns the following error:

UnparseableJsonResponse API Version 2: Failed to parse JSON response string with 'INVALID_ARGUMENT' error: "body: Cannot find field.".

As far as I can tell, it is saying that the body field in the repsonse does not exist in the expected response schema.

The Problem appears to be caused how the Lambda framework in the SDK calls the callback. Current implementation (in src/framework/lambda.ts):

const { status, body } = result
callback(null, {
    statusCode: status,
    body: JSON.stringify(body),
})

There is a trivial fix for this but I'm reluctant to post it here until I can establish my employer's legal position. There is a consensus that raising the issue is acceptable.

Canain commented 6 years ago

The current lambda framework implementation builtin to the library expects the handler to be from lambda's API Gateway. We can explore being able to customize it in the future.

For now, you don't have to necessarily mock the request as long as you know how to extract the JSON body and headers.

Just call app.handler which is a StandardHandler and is the most basic handler that's not dependent on any framework.

It accepts a JSON body as the first argument, a JSON map of header key to header values (as a string or string array), and returns a Promise with the result as a StandardResponse which contains the response body as a JSON and the status as a number.

Using this method, you should be able to use whatever input and output format you like.

igilham commented 6 years ago

Thanks, that does make local testing a little simpler. It'd be easier still, if the framework mechanism supported raw Lambda events in addition to API gateway events. That would make it possible to test locally without adding additional code paths to the app.

It might help to better document the assumptions made by the code. The v1 api docs were quite good so I hope some attention is being paid to improve the v2 docs.

sakthisg commented 6 years ago

Hi Canain, I am novice to this domain, as indicated in the topic, with my AWS Api gateway also.. I am getting an error as below: UnparseableJsonResponse API Version 2: Failed to parse JSON response string with 'INVALID_ARGUMENT' error: ": Root element must be a message.". Can you please let me in detail what code needs to be added in the lambda for converting it. ? It is not clear how to do the conversion by referring the document...Your inputs are appreciated...

The current lambda framework implementation builtin to the library expects the handler to be from lambda's API Gateway. We can explore being able to customize it in the future.

For now, you don't have to necessarily mock the request as long as you know how to extract the JSON body and headers.

Just call app.handler which is a StandardHandler and is the most basic handler that's not dependent on any framework.

It accepts a JSON body as the first argument, a JSON map of header key to header values (as a string or string array), and returns a Promise with the result as a StandardResponse which contains the response body as a JSON and the status as a number.

Using this method, you should be able to use whatever input and output format you like.

Canain commented 6 years ago

@sakthisg Can you provide more logs for your lambda? Like provide the full HTTP response generated by lambda and any error/info logs from your lambda function?

That error by itself doesn't tell much so more info would be useful.

sakthisg commented 6 years ago

@Canain..Thanks for the response. With below code in the lambda function, i am able to get the response in Google assistant properly...

exports.handler = function(event, context, callback) {
    app.handler(event, {}).then((res) => {
        if (res.status != 200) {
            callback(null, {"fulfillmentText": `I got status code: ${res.status}`});
        } else {
            callback(null, res.body);
        }
    }).catch((e) => {
    callback(null, {"fulfillmentText": `There was an error\n${e}`});
    });
}; 

This shall be added in the documentation of the sample code !

Canain commented 6 years ago

We are working on improving the lambda framework support which is being discussed in #219

If that works out, you will be able to use exports.handler = app instead of this work around.

thangta commented 5 years ago

Hi, Did you try?

exports.handler = async (event, context, callback) => { try { await app(event, context, callback); } catch(err) { console.log(err); callback(err);

}

}

sakthisg commented 5 years ago

No.I have not tried. Is this method working for app?