ask-sdk-express-adapter should work with firebase

[bickerdyke]

I'm submitting a...

[ ] Regression (a behavior that used to work and stopped working in a new release)
[ ] Bug report  
[ ] Performance issue
[X] Feature request
[ ] Documentation issue or request
[ ] Other... Please describe:

Expected Behavior

ask-sdk-express-adapter should work with firebase. The problem is, as explained here, the firebase hosting environment automatically inserts a body-parser based on the http-content header.

Current Behavior

When hosting an express app locally, the handlers are working fine, when hosting the same code on firebase functions, requests create an error 'Error in processing request. Do not register any parsers before using the adapter'

Possible Solution

express-adapter should either ignore previous body parsing results if it is providing its own parsing.

Steps to Reproduce (for bugs)

alexahandler.js:

module.exports = function(skillId) {
    return Alexa.SkillBuilders.custom()
    .withSkillId(skillId)
    .addRequestHandlers(
        LaunchRequestHandler,
        MainIntentHandler,
        IntentReflectorHandler)
    .addErrorHandlers(
        ErrorHandler)
    .create();
    }

zmachinevoiceapp .js:

const alexaSkillHandler = require('./alexahandler.js')(apiKeys.ALEXA_SKILL_ID);
const { ExpressAdapter } = require('ask-sdk-express-adapter');
const adapter = new ExpressAdapter(alexaSkillHandler, true, true);

const zmachinevoiceapp = express();
zmachinevoiceapp.post('/', adapter.getRequestHandlers());

module.exports = zmachinevoiceapp;

cloudFuncs.js (exported to firebase)

const functions = require('firebase-functions')
const zmachinevoiceapp = require('./zmachinevoiceapp')

exports.zmachinevoiceapp = functions.https.onRequest(zmachinevoiceapp)

server.js (host for local hosting; works)

const zmachinevoiceapp = require('./zmachinevoiceapp')
zmachinevoiceapp.listen(process.env.PORT || 8080)

1. host locally (if neccessary, create tunnel with ngrok)
2. give as endpoint to alexa testing console: works
3. host on firebase
4. give firebase as endpoint: won't work

Context

I'm trying to add alexa intent handling to an existing actions-on-google backend. That's why the backend should be hosted on firebase functions. Adding it as an additional route in express worked without problems locally after inserting the json-bodyparser only in the actions backend and not in the alexa handler backend.

Your Environment

• ask-sdk: 2.9.0
• ask-sdk-express-adapter: 2.1.0
• express: 4.17.1
• firebase: 7.17.2
• firebase-admin: 9.0.0
• firebase-functions: 3.9.1
• Operating System and version: Win10 and whatever firebase uses to host their hosting environment...

Node.js and NPM Info

• Node.js version used for development: 10
• NPM version used for development: 6.14.1

[ShenChen93]

Hi @bickerdyke ,

Thanks for using our SDK ! The reason we force to use our own parser is due to some edge case caused by json-parser. Sometimes there's decimal number such as 1.0 present in Alexa request, and if developer use json parser, 1.0 will be parsed as 1. Thus in the express adapter, we use the rawParser instead of the Json-parser, and we want to reduce the possibility for developers to hit this edge case.

However, I do think your use case should be a valid use case that we should support. I will add this feature in our backlog. For a quick fix, I think you could remove the first two requestHandlers returned by adapter.getRequestHandlers() function. E.G.

zmachinevoiceapp.post('/', adapter.getRequestHandlers()[2]);

The first requestHandler is the parserChecker and the second requestHandler is the rawParser, which you no longer needed. Let me know if it works for you.

Thanks, Shen

[bickerdyke]

HI. Unfortunately, it didn't work. When I try the [2], I get a "AskSdk.Skill dispatch failed Error, Unexpected token o in JSON at position 1" Is this someone trying to parse an 1.0 version string as integer? :-) However, at least I can reproduce that error on my dev machine...

[ShenChen93]

Hi @bickerdyke ,

Based on the error type, it seems the verification is done successfully. It would be great if you cloud provide the error trace stack which could help me locate the error.

The [2] express requestHandler is defined here, it takes the req provided by express, and call the asyncVerifyRequestAndDispatch function for verification and dispatch. I guess the error is thrown at line 40, where it use JSON.parse function to parse the requestBody. However, error Unexpected token o in JSON at position 1" will be thrown when the provided requestBody is not a valid JSON. Could you please dig into this requestBody and see if it's a valid json ?

Thanks, Shen

[bickerdyke]

I'm still trying to debug deeper into it, but I have troubles getting an error stack or anything sensible into the firebase log. So I tried check locally what's happening when include json-body-parser into the express stack and run through the [2] handler only. You were right. It's line 40. Looks like JSON.parse receives an already parsed object.

Ausnahme: SyntaxError: Unexpected token o in JSON at position 1 at JSON.parse (<anonymous>) at Object.<anonymous> (D:\Marcel\Documents\Projekte\kellerengine\functions\node_modules\ask-sdk-express-adapter\dist\util\index.js:88:60) at step (D:\Marcel\Documents\Projekte\kellerengine\functions\node_modules\ask-sdk-express-adapter\dist\util\index.js:45:23) at Object.next (D:\Marcel\Documents\Projekte\kellerengine\functions\node_modules\ask-sdk-express-adapter\dist\util\index.js:26:53) at fulfilled (D:\Marcel\Documents\Projekte\kellerengine\functions\node_modules\ask-sdk-express-adapter\dist\util\index.js:17:58) at processTicksAndRejections (internal/process/task_queues.js:97:5)

{version: '1.0', session: {…}, context: {…}, request: {…}} context:{Viewports: Array(1), Viewport: {…}, System: {…}} request:{type: 'LaunchRequest', requestId: 'amzn1.echo-api.request.fc7eec23-bb6c-4f94-bbd5-256f6bf8aeaa', timestamp: '2020-08-23T15:08:25Z', locale: 'en-US', shouldLinkResultBeReturned: false} session:{new: true, sessionId: 'amzn1.echo-api.session.7cebfe7b-58e2-47a0-a3a5-723edfa21e11', application: {…}, user: {…}} version:'1.0' __proto__:Object

That's what I'd expect withing firebase servers too, with the parser forced into the pipeline.

[ShenChen93]

Hi @bickerdyke ,

Thanks for the update. If the request is already been parsed, I think you could register a custom handler before the [2] handler, parsing the json object to string. Let me know if it works :)

Thanks, Shen

[bickerdyke]

That feels somehow wrong... but it worked! Thank you!

This code works both during local development and when uploaded to firebase:

const alexaSkillHandler = require('./alexahandler.js')(apiKeys.ALEXA_SKILL_ID);
const { ExpressAdapter } = require('ask-sdk-express-adapter');
const adapter = new ExpressAdapter(alexaSkillHandler, true, true);

// Despite saying not to install any parsers for ask-sdk-express-adapter, install bodyParser.json() for BOTH
// Alexa and Google Assistant Endpoints as firebase will later force the json parser based on http request
// content-type anyway. By putting it here, we have consistent behaviour on firebase servers and locally.
//const zmachinevoiceapp = express().use('/fulfillment', bodyParser.json());
const zmachinevoiceapp = express().use(bodyParser.json());

// workaround for https://github.com/alexa/alexa-skills-kit-sdk-for-nodejs/issues/640
// Remove first handler that checks for preexisting bodyParsers (either we or firebase forced the json-bodyParser into the stack)
// Insert a handler that re-stringifies the already parsed request for the actual skill handler.
var handlers = adapter.getRequestHandlers();
handlers.shift();
handlers.shift();
handlers.unshift((req, res, next) => {
  // guarantee that req.body contains the request as string, not matter if firebase forced bodyparser.json() on us or not.
  if (req.body) {
      req.body = JSON.stringify(req.body);
  }
  next();
});

zmachinevoiceapp.post('/alexaendpoint', handlers); // Install Alexa handlers endpoint
zmachinevoiceapp.post('/fulfillment', dialogflowApp); // install Google Assistant handlers endpoint

Thanks again for helping me come up with the workaround and adding it to the backlog. (Shouldn't be a decimal 1.0 a 1 and if it's supposed to be 1.0 a string? :-) )

[ShenChen93]

Thanks for the updating, glad it's working now :) Closing this issue here for now since it's tracked at our backlog