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

Funnel all intents to same alexa-app intent handler? Please help. #343

Closed roschler closed 6 years ago

roschler commented 6 years ago

I just searched through the alexa-app readme. I did a full search for the word "intent" and read the text surrounding every one of the 85 occurrences of the word "intent". I also searched for the word "unhandled" (0 occurrences). Nowhere could I find out how to either:

1) Catch an unhandled intent name 2) Funnel multiple intents to the same intent handler.

I have several hundred highly specified intents, all handling different semantic and contextual situations. I need to route them all to the same alexa-app intent handler. I can't figure out how to do this. Can someone tell me how I can do either #1 or #2 above?

If not, can someone point me to the source Javascript file in the alexa-app package that looks for the incoming matched intent name and then calls the corresponding intent handler defined during the app.intent() declaration?

I'd hate to after hack the package code just to change the intent handler code to always call the same intent handler, but that's what I'll have to do if there isn't some facility in the package to do this.

matt-kruse commented 6 years ago

For 2, just create a single stand-alone function that handles an intent, with request and response as parameters. Then define all your intents and pass this function as the handler. That way you'll still have lots of different intents, but just one handler function.

But why do many intents? Since a single intent can have hundreds or thousands of different utterances attached to it, why not combine all those intents into just a few, with lots of different phrases?

For 1, there is no way to trap a named intent that does not have a handler function. Because, this shouldn't even be possible. Since the schema should be generated from your skill code, and the code defines the intents, it would be impossible to have an intent that does not have a handler. If you are doing these two things separately, then that could happen, in which case you will get an exception thrown that there was no handler defined.

If you're looking for the more general handler, where Alexa didn't recognize the text spoken and didn't match it to an intent, and you want to handle that case, then that's not current a feature of Alexa at all. But it will be coming.

roschler commented 6 years ago

@matt-kruse Hi Matt. I'm a veteran NLP developer so I already have a large body of code that handles tons of user input variations. I just need to get the input coming from Alexa to the code.

I'm OK now. I've looked at the index.js file for alexa-app and found the code that throws an error if a named intent handler could not be found:

      if ("IntentRequest" === requestType) {
        var intent = request_json.request.intent.name;
        if (typeof self.intents[intent] !== "undefined" && typeof self.intents[intent].handler === "function") {
          if (self.intents[intent].isDelegatedDialog() && !request.getDialog().isCompleted()) {
            return Promise.resolve(request.getDialog().handleDialogDelegation(request, response));
          } else {
            return Promise.resolve(self.intents[intent].handler(request, response));
          }
        } else {
          throw "NO_INTENT_FOUND";
        }
      } 

I'm just going to add code that instead of immediately throwing an error if a named intent is not found, looks for an intent handler named "unhandled" first and calls that handler if it exists. If not, then it throws the handler.

roschler commented 6 years ago

Closing this issue now since I have my solution. I added this to the alexa-app index.js file. I just tested it and it works:

      if ("IntentRequest" === requestType) {
        var intent = request_json.request.intent.name;
        if (typeof self.intents[intent] !== "undefined" && typeof self.intents[intent].handler === "function") {
          if (self.intents[intent].isDelegatedDialog() && !request.getDialog().isCompleted()) {
            return Promise.resolve(request.getDialog().handleDialogDelegation(request, response));
          } else {
            return Promise.resolve(self.intents[intent].handler(request, response));
          }
        } else {
          // ROS: Add support for calling a intent handler named "unhandled" if it exists.
          var unhandledIntentHandlerName = "unhandled";

          if (typeof self.intents[unhandledIntentHandlerName] != 'undefined' && self.intents["unhandled"] != null)
            // Call the unhandled intent handler.
            return Promise.resolve(self.intents[unhandledIntentHandlerName].handler(request, response));
          else
            // Throw an error.  No suitable intent handler found.
            throw "NO_INTENT_FOUND";
        }
      }
matt-kruse commented 6 years ago

That's one way to fix it, but you won't be able to upgrade alexa-app since you've modified its internals.

I guess I still don't understand your problem well enough, because this really should be unnecessary. This doesn't feel like the right solution for any situation.

roschler commented 6 years ago

@matt-kruse I appreciate your help and I wouldn't expect you to modify alexa-app for my situation since it is extremely different from anyone else that will use your package. Thanks again for your help.

dblock commented 6 years ago

I think a feature in alexa-app that customizes what happens when an intent is not matched (other than throwing an exception) isn't a bad idea, so I welcome a PR and we can discuss it over that code?

matt-kruse commented 6 years ago

I would quickly submit a patch to accomplish this, but to be honest this project has become almost too cumbersome to contribute to. Quick, simple enhancements get buried in tooling, tests, type definitions, etc. I'd love it if this could move faster with less friction so we could adapt to the rapidly-changing Alexa environment. There are lots of new features just on the horizon that will need to be supported with code changes!

dblock commented 6 years ago

Matt, what are you proposing?

matt-kruse commented 6 years ago

I would like to see PR's merged that do not have corresponding tests, or that maybe cause the automated checks to fail, if the code is known to be good. Tests can be added later, etc. Because in the end, all the build and automation doesn't matter, it's whether index.js runs as intended.

Ideally, it would be great to have everything 100% green and good with each commit. But for people like me, I just can't afford that much overhead, I've got lots of other things to move forward. I understand the value, not saying it's bad, just saying it prevents me from contributing fixes that I know would help others.

dblock commented 6 years ago

What you call overhead just moves the burden on others to debug your code when it doesn't work or when your code breaks someone else's code accidentally. I don't have time for that, so personally, I am not interested in participating or using a library that doesn't ensure that existing features weren't broken or new features added actually work at the time when they are written.

You as the originator of this project can take it any direction, but I encourage you to look at the hard work that was put in here by 37 contributors including some very big things by @ajcrites @kobim or @lazerwalker since your last major commit in 2016 and how smoothly things have gone by ensuring a level of quality that burdens the developers to write tests with their new features.

matt-kruse commented 6 years ago

I understand, and I'm fine with it staying that way. I like it being that robust. For me, personally, I can't keep up with that level of complexity in the process. I don't understand all the layers that are in place now. And even though I have advance access to new Alexa API's and can code them into alexa-app to have ready the day they are released to the public, I find that I can't do so because of the burden of tests, etc. My last simple change to add the .on() handler took a lot of time and patience for just a few lines of code. I wish I had time to invest in understanding all of it, but I just don't.

So I'm fine with it staying how it is, but it will depend on others to implement new API features, and will significantly lag behind the coming API's from Amazon. For example, I'm currently coding a layer of support for the Gadgets API, but I'm doing it in a module that I will release separately, because I won't be able to meet all the requirements for contributing to this code base.

I guess the real question is, how can we be accommodating to developers who want to contribute good code to the project, but lack the knowledge/experience required to write tests, declare types, etc? Or are contributions limited to only those who understand the tooling in place? The latter is not my preference, but I understand it, and I will continue to do what I can in that mode.

dblock commented 6 years ago

Thanks. I think there're a few things I can suggest.

1) Make pull requests that don't have tests and have working code and just add that you don't have time to finish the PR and hopefully someone can jump in and do it. 2) Try to do just a tiny bit of extra work. For example, I personally find that working with tests is much faster than without, I get more done in less time by writing a failing test first. 3) Find time to consider learning the new tooling, like TypeScript, it will change a lot of how you think. Using things like VSCode with TS is just amazing. Keep an open mind.

roschler commented 6 years ago

@matt-kruse Will you be making your changes to a fork of the project instead? That Gadgets API sounds interesting.