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

Fix default response template #301

Merged
merged 2 commits into from
May 28, 2018
Merged

Fix default response template #301

merged 2 commits into from
May 28, 2018

Conversation

patrickheeney
Copy link
Contributor

@patrickheeney patrickheeney commented Sep 4, 2017

First time installing serverless-offline and a few bugs on my functions. The response template was not being set because fep.response was undefined for me. It was missing a check, similar to here. Also this.responseContentType is undefined so switched it to the correct context.

The second commit fixes another bug after I got it to load the default response. Because there is an extra line break \n it caused this line to return {'application/json': '\n' }. This caused the response to get sent into Velocity and since the template is just \n, returns a blank response. Removing the line break fixes this but maybe we should strip trailing \n before checking if its blank?

@dherault
Copy link
Owner

Thank you for your PR, please consider #304

@dherault
Copy link
Owner

Duplicate of #285 ? Thank you @patrickheeney anyway

@dherault dherault closed this Sep 18, 2017
@dherault
Copy link
Owner

Thank you for your contribution! v3.16.0

@patrickheeney
Copy link
Contributor Author

patrickheeney commented Sep 18, 2017 via email

@patrickheeney
Copy link
Contributor Author

@dherault please see commit df2f460 and compare to 6f6a628 . They do not cover the same thing and the issue still remains.

@dherault dherault reopened this Sep 19, 2017
@dherault
Copy link
Owner

My bad @patrickheeney :)

@dherault dherault merged commit 7a7ea40 into dherault:master May 28, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants