fix: deleting an island wedges the dev server#3883
Open
aiibe wants to merge 1 commit into
Open
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The Problem
Whenever you deleted a file from the
islands/folder while runningdeno task dev, the whole app would crash with a permanent HTTP 500 error until you completely restarted the server. Because Fresh auto-discovers everything in that folder, this happened even if the island you deleted wasn't actually being used anywhere in the app.What was happening?
There were basically two gaps that combined to wedge the server:
routes/directory, so deleting an island didn't actually tell the server snapshot that it needed to rebuild.The Fix
I made two changes that work together to solve this:
loadhandler to completely clear out and rebuild the island map from scratch on every reload, rather than just adding to it. I also threw in a quick cache to make sure the generated island names stay stable and don't drift between rebuilds.Testing
I added a new test (
dev_server_test.ts) that specifically mimics this bug. It loads up a page, deletes an unused island, and verifies that the server successfully recovers and keeps happily returning 200s instead of crashing.