Not reloading on deletes

[d4h0]

Hi,

I guess most people who use devserver with the 'reload' option generate the content of their website automatically.

At startup, for a clean build, I currently delete the folder that devserver watches, and I restart my app automatically via cargo watch when the code changes.

Unfortunately, that triggers a reload which ends in a 404 error because the file does not exist yet.

I think it would make sense not to reload on deletes.

Basically, you'd just return false from this match arm.

This would also be consistent with how renames are handled.

What do you think?

[d4h0]

Btw. at the beginning, I didn't run into this problem. I guess my app is a bit slower now, so the event debouncing doesn't catch this anymore.

In case, you can't reproduce this issue.

[d4h0]

You probably also could simplify the code a little:

    use notify::DebouncedEvent::*;
    match event {
        Error(..) => panic!(), // I think, a proper error message should be shown here  
        NoticeWrite(..) | NoticeRemove(..) | Remove(..) | Rename(..) | Rescan => (),
        Create(_) | Write(_) | Chmod(_) => 
            // A blank message is sent triggering a refresh on any file change.
            // In the future a filepath be sent here.
            // If this message fails to send, then likely the socket has been closed.
            if send_websocket_message(&stream, "").is_err() {
                break;
            }
    }

(untested)

[kettle11]

This is a really good catch. This issue is a pretty big workflow pitfall:

If say index.html is deleted and a reload occurs then the page 404s and reloads can no longer occur so the user has to manually refresh the page. This is probably a pretty common issue.

Your proposed fix looks like the right one.

---

Side note:

This is also highlighting the need for a better 404 error. Right now when devserver returns a 404 the browser produces a result that's pretty similar to what you'd see if devserver wasn't running at all. It's easy to miss that it's a 404.

Additionally if devserver returned a 404.html page for missing .html files then the 404 page could also embed the reloading code. Then if the missing .html file were created the next reload would load the proper file.

[d4h0]

Additionally if devserver returned a 404.html page for missing .html files then the 404 page could also embed the reloading code. Then if the missing .html file were created the next reload would load the proper file.

Yes, this would be a great addition.

Something else useful would be, to show all available files and directories on the 404 page. This is what the dev server of Gatsby.js does. This way typos can be corrected fast, and it's easy to switch to all pages without much typing (useful to switch to pages that are not commonly linked).

[kettle11]

I've implemented your proposed fix in this change (disabling reload for deleted files): https://github.com/kettle11/devserver/commit/6c76e4307b86312989c0ef7b7d7629b1720f9198

Thanks again! :)

I've opened a new issue about the 404 page as it's something that should eventually be done: https://github.com/kettle11/devserver/issues/12

Showing the available files and directories would be nice in some form eventually. As you said it's certainly handy to quckly figure out what's setup incorrectly.

[d4h0]

Thank you, @kettle11.

I've installed devserver from the repo, but now live-reloading doesn't work anymore – does it still work for you?

[kettle11]

@d4h0 Oh no that's not good. It's still working for me.

I tried to reproduce it on my computer but I could not.

Fortunately not much has changed so there are two possibilities I can think of:

• Sending an empty websocket message doesn't work on your setup for whatever reason
• Perhaps your OS / setup sends a file delete / file create instead of a modified? (This seems unlikely)

I made this branch with a potential fix for the first issue: https://github.com/kettle11/devserver/tree/reload_fix. Instead of sending an empty WebSocket message it sends the word "reload".

If you have time would you mind cloning that branch to see if it works for you?

[d4h0]

At the moment I reload my app with every change, delete the build folder, and then re-create everything. My assumption was that this would work because there is a 'delete' event (which is ignored), and then a 'create' event (which should trigger the reload).

Could it be, that the newly created file isn't associated anymore with the page?

The odd thing is, that this exact setup worked before without a problem.

If you have time would you mind cloning that branch to see if it works for you?

Yes, I will try that tomorrow, thank you!

[d4h0]

I've tested the 'relaod_fix' branch, but this didn't fix the problem. I've switched back to master now.

In my code I have the following:

    if output_dir.exists() {
        fs::remove_dir_all(output_dir)?;
    }

    fs::create_dir(output_dir)?

After looking at your code, I thought it could be a problem that the watched directory is completely deleted. That could be a problem, if Notify uses some kind of filesystem ID instead of the path here.

So I changed the code to the following, to only delete the content of the watched directory:

    if output_dir.exists() {
        for entry in fs::read_dir(output_dir)? {
            let entry = entry?;
            let path = entry.path();
            if path.is_dir() {
                fs::remove_dir_all(path)?;
            } else {
                fs::remove_file(path)?;
            }
        }
    }

With this – for some strange reason – I get the 404 error again when I change a file.

Do you have any idea what the problem could be?

I don't believe that I have some special setup that most people don't have, so I guess other users could run into this problem as well.

In cases like this, logging is very helpful – would you be willing to add that?

With a feature flag for the code that initiates the logger, this would not increase build time for others. The log macros of the log crate also can be deactivated via feature flags.

On the other hand, even without feature flags, the addition of logging (for example with env_logger) shouldn't increase compile time much (and would make it much easier to activate logging for the binary).

I think logging is very helpful for debugging, and for documenting the code.

Btw.: I saw that you clone &str sometimes. As far as I know, you don't have to do this because &str is 'copy' (see this playground)

So you can delete lines like this one.