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.

Sign up for GitHub

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

Jump to bottom

Race condition between can-serve and live-reload #62

Closed
matthewp opened this issue Sep 25, 2015 · 8 comments
Closed

Race condition between can-serve and live-reload #62

matthewp opened this issue Sep 25, 2015 · 8 comments
Assignees
@matthewp
Labels
bug

Comments

@matthewp
Copy link
Contributor

Since can-serve and live-reload are launched at the same time, if can-serve loads the application first it will try to send a WebSocket request to the live-reload server which hasn't started yet. We need to wait until the live-reload server is ready before trying to start can-serve.
@matthewp matthewp added the bug label Sep 25, 2015
@daffl
Copy link
Contributor

daffl commented Sep 25, 2015

When is that the case? I would think until you pull the page up in your browser live-reload should be up and running?

The problem is that we have to have the rather hacky way of spinning up live-reload as a child process (see https://github.com/canjs/can-ssr/blob/master/bin/can-serve#L31) because we can't load the app a second time (#42). If there would be some way to share the loaded application between ssr and live-reload we could wait until the live-reload server is listening.

@matthewp
Copy link
Contributor Author

When is that the case? I would think until you pull the page up in your browser live-reload should be up and running?

live-reload happens in the server code as well. The browser will still live-reload but the server will not, if can-serve loads before the live-reload server is started.

@daffl
Copy link
Contributor

daffl commented Sep 25, 2015

Ah ok. We could have live-reload print out that it is listening and wait for that before starting the server. The right fix however would be to share the application and run everything in the same process.

@matthewp
Copy link
Contributor Author

live-reload does print out what port it is listening on. That is the fix I was thinking of as well.

@matthewp matthewp self-assigned this Sep 25, 2015
@matthewp
Copy link
Contributor Author

I'm going to take this one, will wait for the message to be printed.

@justinbmeyer
Copy link
Contributor

That sounds like a dangerous fix

@justinbmeyer
Copy link
Contributor

Let's talk about this before resorting this. I'd much rather make can-serve re-try the connection to live-reload after a short timeout. It might miss messages, but that's ok.

@matthewp
Copy link
Contributor Author

I can listen to the close event on the websocket and retry.

matthewp added a commit that referenced this issue Sep 28, 2015
@matthewp
Specify liveReloadAttempts
c3fecf5 
This uses the new retry capability added in
stealjs/live-reload#18

Sets liveReloadAttempts to 3, so there will be 2 retries after the
connection fails to establish.

Fixes #62
@matthewp matthewp mentioned this issue Sep 28, 2015
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@matthewp matthewp

Labels
bug
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@justinbmeyer @daffl @matthewp