kevin-mitchell / alexa-app

Set of classes to make creating Amazon Alexa Skills Kit (ASK) applications easier with Laravel and Lumen
MIT License
97 stars 47 forks source link

Alexa and Alexa Route Facades do not work...laravel5.4 #24

Closed chawker21 closed 6 years ago

chawker21 commented 7 years ago

Ive gone over everything and I cannot get a AlexaRoute::launch to work it seems this is the same issue I had about a year ago when I tried this with 5.3.

I am however able to get a response as you do in your video but by using ROUTE::POST using the postman app I send a post request with the launch request I get from the alexa skill kit and I get {"version":"1.0","response":{"shouldEndSession":true}} but honestly I dont know if that is saying anything or not. I havent been able to write a response where it will return anything more.

scottlaurent commented 7 years ago

I think something is broken here as well. I can't even get the routes to properly register.

In other words

AlexaRoute::launch('/alexa-end-point', 'App\Http\Controllers\AlexaController@getLaunch'); << THIS DOES NOT WORK. In fact, it's not even showing up in the route list.

Getting a Not FOund Exception in Postman.

How can something be so fundamentally broken here?

S43534 commented 7 years ago

It works fundamentally fine for me... The routes don't show up in route:list either (due to the routing concept of the package), but it works.

Do you maybe use Route::group? Seems like there might be an issue with that. See https://github.com/develpr/alexa-app/issues/20.

scottlaurent commented 7 years ago

no. i was about to post again. Can someone paste a fully working simple route group that WORKS in 5.4? I just did a new install, installed only this package, and right out of the gate, it gives me a 404. So I copied the #20 as well and again, 404. Nothing I can do, any combination, seems to route properly. 404 on every request. Route::post works fine, but that defeats the entire purpose obviously.

scottlaurent commented 7 years ago

To be clear, this is the result immediately after a new install.

// DOES NOT WORK
AlexaRoute::launch('test', function() {
   dd('test');
});

// WORKS
Route::post('test', function() {
   dd('test');
});

// DOES NOT WORK
AlexaRoute::intent('test', 'intent', 'App\Http\Controllers\AlexaController@postIntent');

// WORKS
Route::post('test', 'App\Http\Controllers\AlexaController@postIntent');
S43534 commented 7 years ago

Your route is fine, mine are exactly the same and work. However, a simple POST to this route will not work as it misses information that the router of this package needs to identify it as an Alexa request. Did you already try it from the Amazon developer console? Put return Alexa::say("This is a test."); into the controller method and call it from Amazon. It will work.

scottlaurent commented 7 years ago

I can't reach the controller correctly because the AlexaRoute does not work.

The above AlexaRoute routes don't work. They may work on your install which has something else going on other than a fresh install of 5.4 plus the install instructions located here.

My point above was just to explain that Route:post works, so the install of Laravel and general routing works.

However AlexaRoute does not work, in any situation. Again, this is a fresh install, done twice now over the last 30 minutes.

At this point, I will probably need to just abandon using this package which is unfortunate because it seems like the only one. But the fact is, it IS NOT WORKING on a new install of 5.4.

S43534 commented 7 years ago

Disclaimer: I am just a user of this package and not the developer, and I had issues myself as you might see in other threads.

I am working on a clean install of Laravel 5.4. No extras installed or configured. And it works. You cannot just say "it is a fact that is is not working" when you just can't get it running. My installation shows exactly the same behavior as yours. Routes are not shown. A simple POST will not work to test it. Did you even try to call it from Amazon?

When debugging I have built a middleware (based on someone else's work):

namespace App\Http\Middleware;

use Closure;
use Illuminate\Support\Facades\Log;

class LogAfterRequest {

    public function handle($request, Closure $next)
    {
        return $next($request);
    }

    public function terminate($request, $response)
    {
        Log::info('app.request', ['headers' => $this->getallheaders(), 'request' => $request->all()]);
        Log::info('app.response', ['response' => json_encode($response->content())]);
    }

    private function getallheaders()
    {
        if (!is_array($_SERVER)) {
            return array();
        }

        $headers = array();
        foreach ($_SERVER as $name => $value) {
            if (substr($name, 0, 5) == 'HTTP_') {
                $headers[str_replace(' ', '-', ucwords(strtolower(str_replace('_', ' ', substr($name, 5)))))] = $value;
            }
        }
        return $headers;
    }
}

Call your server from Amazon and have a look at your log - you will see how a POST request has to look for this package to work properly.

Sebastian

scottlaurent commented 7 years ago

if you are posting the same json packets from postman and bypassing cert checks, it should be an acceptable method of testing, no?

The person who opened this ticket appears to be having the same issues.

I did check the request logs from amazon and I can see the request coming in. And it's replying with a 404.

If you are got this working from a fresh install, then you are doing something maybe either undocumented or something that is entirely not clear from the install instructions.

And, to answer your question, I also tested constantly from the console and finally got sick of the errors, so I resorted to postman with the same json packets, and did not do cert checking, to recreate the same request.

S43534 commented 7 years ago

Just out of curiosity I set up a completely new installation (directly on a prod server) according to the documentation of this package, set up an Amazon skill, and it works.

I did not more than:

composer create-project --prefer-dist laravel/laravel alexa-test
cd alexa-test/
chown -R envoyer:www-data alexa-test
chmod -R 775 alexa-test/storage/
composer require develpr/alexa-app
nano config/app.php 
nano app/Http/Kernel.php 
nano routes/web.php 
php artisan make:controller AlexaController
nano app/Http/Controllers/AlexaController.php
php artisan vendor:publish
nano config/alexa.php 

alexa

scottlaurent commented 7 years ago

what you did is exactly what I did except i used dev-develop.

I get the requests in the same as well, no errors when the app bootstraps, but just returns a response 404: (NotFoundHttpException in RouteCollection.php line 161)

S43534 commented 7 years ago

Then I'm running out of ideas... Just to make sure.

My endpoint in Amazon: https://xxxx.xxxx.de/alexa

My intent:

{
    "intents": [
      {
            "intent": "TestSkill"   
        }
    ]
}

My route: AlexaRoute::intent('/alexa', 'TestSkill', 'App\Http\Controllers\AlexaController@test');

My method test:

class AlexaController extends Controller
{
    public function test()
    {
        return Alexa::say('Test. Hi.');
    }
}

Sample utterance: TestSkill Test the skill

Utterance to test it: "test the skill"

ArranJacques commented 6 years ago

So I struggled with this exact issue for several hours and after spending ages digging around the code I eventually discovered the issue and got a fix in place.

I have my route defined as follows:

AlexaRoute::launch('/alexa/session/launch', \App\Http\Controllers\Alexa\SessionController::class . '@postLaunch');

I had to use the full class definition as the usual shorthand namespace didn't work.

I was testing this route using a REST Client to make a POST request to /alexa/session/launch with the request body set as:

{
  "version": "1.0",
  "session": {
    "new": true,
    "sessionId": "xxx",
    "user": {
      "userId": "xxx
    }
  },
  "request": {
    "type": "LaunchRequest",
    "requestId": "xxx"
  }
}

This request would 404 regardless of what I did.

After digging around I found the problem was that the route was failing to match here https://github.com/illuminate/routing/blob/ccc341130083a447f793c7621b17176960276437/Matching/UriValidator.php#L21.

The problem was that the $route->getCompiled()->getRegex() statement was generating the regexp #^/alexa/session/launchLaunchRequest$#s.

Basically LaunchRequests was getting appended onto the end of the regex so what was getting run was return preg_match('#^/alexa/session/launchLaunchRequest$#s', '/alexa/session/launch'), which obviously failed, so the route was 404ing.

I changed my test request to post to /alexa/session/launchLaunchRequest and boom, it worked.

The offending line of code is here. https://github.com/develpr/alexa-app/blob/478d792c793f903ab001ac844dbf8c7bacb516b5/src/Http/Routing/AlexaRoute.php#L90

So basically when comparing the request URI to the URIs defined when using the AlexaRoute class the request types is getting appended onto the end all the defined routes, meaning they never match.

For reference I was testing this in a fresh install of Laravel 5.5.

I hope that helps some people!

Schnoop commented 6 years ago

Hey @ArranJacques

thanks for have a look. You fix works well, but only if your have a single intent request. If there a more than one request, and you remove the $this->getRouteIntent() for the uri method, you'll overwrite the previous url that has been registered, as every indent request has the same url.

I fixed the problem by re-adding the "$this->getRouteIntent()", but keep commented out the lines in the "compileRoute" method.

I hope that helps for those who have multiple indent requests.

amenk commented 6 years ago

Okay so we have to remove the lines

protected function compileRoute()
{
    //todo: we're doing this to support < 5.4 - remove this in 5.5
    if(is_callable("parent::extractOptionalParameters")){
        return parent::compileRoute();
    }

but how can we do this for 5.5 and still support 5.4 ?

amenk commented 6 years ago

found a way if (version_compare(\Illuminate\Foundation\Application::VERSION, '5.5.0') < 0) {