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

Reload FastHTML app after server error #642

Closed
wants to merge 1 commit into from

Conversation

dgwyer
Copy link
Contributor

@dgwyer dgwyer commented Jan 28, 2025

Proposed Changes
For a FastHTML app that has live reload enabled, after a server error occurs (e.g. triggered by a typo in your app source code) the web socket does not automatically reconnect. This PR fixes that. When the server restarts from an error the WS is automatically reconnected.

See this related issue for more details.

Types of changes
What types of changes does your code introduce? Put an x in all the boxes that apply:

  • Bug fix (non-breaking change which fixes an issue)

Checklist
Go over all the following points, and put an x in all the boxes that apply:

  • My code follows the code style of this project.
  • I am aware that this is an nbdev project, and I have edited, cleaned, and synced the source notebooks instead of editing .py or .md files directly.

Additional Information
Any additional information, configuration or data that might be necessary to reproduce the issue.

@dgwyer dgwyer changed the title Reconnect web socket connectin after server error Reconnect web socket connection after server error Jan 30, 2025
@dgwyer dgwyer changed the title Reconnect web socket connection after server error Reload FastHTML app after server error Feb 5, 2025
@curtis-allan
Copy link
Contributor

Thanks for taking the time to craft this PR @dgwyer. Definitely agree that this would be a nice addition to local development.

A couple of notes:

  • Clever use of the fetch request to check for page status vs the ws route!
  • Noticed you changed reload_attempts but it's not being passed to the formatted string, was there a reason for this?
  • The changed code is quite large for what it does

You have some great functionality here, tested it out locally and it works well. I might close this PR and make a new one with some tweaks while keeping it functionally similar.

Thanks again for the work on this enhancement, we will credit your efforts in the release notes :)

@dgwyer
Copy link
Contributor Author

dgwyer commented Feb 8, 2025

@curtis-allan Thanks for the feedback!

Noticed you changed reload_attempts but it's not being passed to the formatted string, was there a reason for this?

I'm not sure if I remember now. 🙂 Maybe I tried something that I didn't follow-up with.

I also tried to reduce the reload_interval in a separate test (which I didn't submit a PR for). I really wanted to improve the reload speed to improve the UX when live reload is enabled. Perhaps you could also look into this in your PR?

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