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

Bring back default http server wrapper #460

Merged
merged 2 commits into from
Jul 31, 2022

Conversation

walaszczykm
Copy link
Contributor

The micro has a breaking change in its minor version upgrade from 9.3.4 to 9.4.0 related not to default wrapping the "serve" function result into http.Server instance, which leads to "server.listen is not a function" error.

The difference can be seen by comparing the source and README:

This PR proposes reverting this part of the code and readme to the state from version 9.3.4

@walaszczykm
Copy link
Contributor Author

Hey @leerob, sorry for the confusion, I saw that tests are hanging ad dug a little bit into why. Turns out the _test-utils.js was wrapping micro into new http.Server instance. I have adjusted it accordingly.

I feel quite hesitantly as those are changes from 3 years ago, and now I am reverting them, but I see they all were all rolled out as a canary release until now, the new 9.4.0 version. If you feel that this new way of consuming micro programmatically is better suited, then maybe going with it in a dedicated major release would also be an option.

@leerob leerob mentioned this pull request Jul 31, 2022
@leerob leerob merged commit 6b973cb into vercel:main Jul 31, 2022
@walaszczykm walaszczykm deleted the fix-http-server-wrapper branch July 31, 2022 18:40
@Bessonov
Copy link

Bessonov commented Aug 3, 2022

From my point of view, it would be nice to get rid of http.Server. It makes micro usable with another implementations like https.createServer.

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.

3 participants