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

created a server class to have a single instance of hono server running #2

Merged
merged 4 commits into from
May 20, 2023

Conversation

Shofiya2003
Copy link
Contributor

@Shofiya2003 Shofiya2003 commented May 17, 2023

Parallel restart led to EADDRINUSE error. Solved this issue using singleton server, created a Server class to have a single instance of hono server running at a time. Added init and close function to the class.

@barelyhuman
Copy link
Owner

barelyhuman commented May 17, 2023

Cool, you could however move the entire init and close also in the kernel/hono.js then instead of having them in cli.js

Example it'll just be server.init() and server.close() in the cli.js and the kernel.js handles the creation and shutting down for you

prev/cli.js Outdated
@@ -288,22 +271,9 @@ function normalizeConfig(config) {

async function queueRestart() {
const mod = await import('./kernel/index.js')
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I remove direct import from here and add getServer function in config?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope

@barelyhuman
Copy link
Owner

So, the init phase was just supposed to be for initialising the server start and listening phase, the cli's original intention was to stay as is. I'll add the modifications once I'm done with the other to help understand

@barelyhuman barelyhuman merged commit 5e85bf9 into barelyhuman:main May 20, 2023
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