Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Test hangs in verifier #152

Closed
dblock opened this issue Feb 4, 2017 · 6 comments
Closed

Test hangs in verifier #152

dblock opened this issue Feb 4, 2017 · 6 comments

Comments

@dblock
Copy link
Collaborator

dblock commented Feb 4, 2017

This test hangs for no reason: https://github.com/dblock/alexa-app/blob/test-hangs/test/test_alexa_verifier_middleware_hangs.js, the middleware never fires on('end', function() { }), not sure what's going on.

@dblock
Copy link
Collaborator Author

dblock commented Feb 4, 2017

@mreinstein you might be able to explain this one, I'm probably missing something obvious

dblock added a commit to dblock/alexa-app that referenced this issue Feb 4, 2017
@dblock
Copy link
Collaborator Author

dblock commented Feb 4, 2017

So ... we need a body parser after the middleware. Does this mean #155?

@mreinstein
Copy link
Contributor

the alexa-verifier-middleware already effectively acts like a JSON body parser. see: https://github.com/alexa-js/alexa-verifier-middleware/blob/master/index.js#L31

I think the logic should be:

if(checkCert) {
  install middleware
} else {
  install regular json body parser
}

the if clause is already implemented. just need the else

dblock added a commit to dblock/alexa-app that referenced this issue Feb 4, 2017
@dblock
Copy link
Collaborator Author

dblock commented Feb 4, 2017

Ok, cool, updated.

I am also not moving body-parser into dependencies just like we do with express. I assume this is correct?

@mreinstein
Copy link
Contributor

I am also not moving body-parser

if the code is using body-parser why would it not be a dependency?

dblock added a commit to dblock/alexa-app that referenced this issue Feb 4, 2017
dblock added a commit to dblock/alexa-app that referenced this issue Feb 4, 2017
@dblock
Copy link
Collaborator Author

dblock commented Feb 4, 2017

We only use it in the express scenario, but you're right.

@dblock dblock closed this as completed in 8e0e6fe Feb 5, 2017
dblock added a commit that referenced this issue Feb 5, 2017
Fix #152: mount a JSON body-parser after verifier middleware in Express integration.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants