alexa-js / alexa-app

A framework for Alexa (Amazon Echo) apps using Node.js
https://www.youtube.com/watch?v=pzM4jv7k7Rg
MIT License
1.03k stars 212 forks source link

Suggested change to docs regarding asynchronous calls in intent handlers #341

Open roschler opened 6 years ago

roschler commented 6 years ago

I used these docs to develop my intent handler that requires an asynchronous call to an external server:

https://www.npmjs.com/package/alexa-app#asynchronous-handlers-example

The docs give this code sample as an example of an async call inside an alexa-app intent handler:

app.intent("checkStatus", function(request, response) {
  // `getAsync` returns a Promise in this example. When
  // returning a Promise, the response is sent after it
  // resolves. If rejected, it is treated as an error.
  return http.getAsync("http://server.com/status.html").then(function (rc) {
    response.say(rc.statusText);
  });
});

I think the docs need a fully fleshed out Promise example. In order to get the body of my Promise working I had to make an explicit call to resolve(response) when I was done. Without that call, alexa-app returned an ERROR to Alexa with an empty payload and the end session flag set to true.

Perhaps something like this?:

/**
 * Sample promise that makes an async call that can be called inside an alexa-app intent handler.
 *
 * @param {Object} request - An alexa-app request object.
 * @param {Object} reponse - An alexa-app response object.
 * @return {Promise}
 */
function asyncCallInsideIntent_promise(request, response)
{
    // Build a new promise object that calls the dialog server and process the result before
    //  returning our response to Alexa
    return new Promise(
        function(resolve, reject)
        {
            try
            {
                // Validate input parameters.
                if (request == null)
                    throw new Error('The request object is unassigned.');

                if (response == null)
                    throw new Error('The response object is unassigned.');

                // Make the async call.  Do not make any explicit return calls.  See the
                //  comment above the resolve() call in the "then" block below.  The
                //  fake async call below simply returns some text to say to Alexa.
                doAsyncCall()
                .then(function(newText)
                {
                    // Resolve with the response or the intent will
                    //  end up returning an empty JSON payload to Alexa with the
                    //  end-session flag set to TRUE.
                    resolve(response);
                })
                .catch(function(err)
                {
                    // Convert the error into a promise rejection
                    reject(err);
                });
            }
            catch (err)
            {
                // Convert the error into a promise rejection
                reject(err);
            }
        });
}

If you want me to fork the repo and make a pull request I'm happy to do that, but I'd like to get your comments first before doing that.

dblock commented 6 years ago

Please make a PR, we can look at the actual change.

roschler commented 6 years ago

@dblock Just did:

https://github.com/roschler/alexa-app/pull/1

gnickm commented 6 years ago

+1 on this doc change. I spent several hours struggling with the promise resolve until I read the above. Thx @roschler !

dblock commented 6 years ago

Thanks! Please make a PR into the root project (aka here) @roschler.