Fix Micro server handler #401
Conversation
|
@spicydonuts: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Meteor Contributor Agreement here: https://contribute.meteor.com/ |
There was a problem hiding this comment.
@spicydonuts thanks a lot for the PR, this looks great! Since the PR is supposed to fix issues, please make sure to add tests for what you changed. That way we can be sure that it works and stays working.
|
Good idea. I've added |
|
And now I'm going to go PR |
|
Not really sure why the build failed.. it doesn't appear related to these changes. The node 6 build passed but the node 4 and 7 builds failed with |
|
I'm tempted to touch one of the files to get Travis to re-run. Anyone able to do that without a new commit? |
|
Hi @spicydonuts I've restarted the CI jobs in question. Let's hope they pass this time! 🙂 |
|
Success! 🎉 |
The handler was not returning `httpRunQuery`'s Promise, causing micro to send empty responses and leaving Node with unhandled Promise rejections when that Promise tried to write headers. I also used `return` and `throw` instead of manipulating the response directly. This allows wrapping middleware to intercept responses if desired.
Adding @types/micro caused the test to stop compiling. Adjusted the import to fix.
|
Is there anything blocking this merge? I fixed the merge conflict that's appeared in the meantime. |
|
Hey @spicydonuts, thanks for the reminder, I'll take another look today! |
There was a problem hiding this comment.
Hey @spicydonuts sorry for the delay and thanks for the PR! Everything looks good to me, I'll merge it now! 🙂
|
is this released? 😛 |
|
@luisrudge yes |
The handler was not returning
httpRunQuery's Promise, causing micro to send empty responses and leaving Node with unhandled Promise rejections when that Promise tried to write headers.I also used
returnandthrowinstead of manipulating the response directly. This allows wrapping middleware to intercept responses if desired.