HotReload is not working in ASP.NET Core in VS2022 or dotnet watch run when ServiceWorkers are installed/running

[ChaosEngine]

Is there an existing issue for this?

• [X] I have searched the existing issues

Describe the bug

When created brand new ASP.NET Core 6 project and added ServiceWorker JavaScript code (for caching, offline handling etc.) HotReload functionally is not loading. IMVHO it is related to the fact that key messing ingredient SignlaR script:

    <script src="/_framework/aspnetcore-browser-refresh.js"></script>

is not loaded. Why ?? don't know. So in order to make it behave properly user is obliged to load manually this script (for example by including it in _Layout) After that all is back to normal again. The script is simply not emitted when Service Worker are installed and running.

Same behavior is observer when run with Ctrl-F5 in VS2022 and through CLI -> dotnet watch run

Rerpro project demonstrating the issue

Expected Behavior

When ServiceWorker is installed and running hotReload should work out of the box in VS2022 and through dotnet watch run

Steps To Reproduce

https://github.com/ChaosEngine/NoHotReloadWithServiceWorker

Important pieces: NoHotReloadWithServiceWorker\wwwroot\sw.js - Service worker NoHotReloadWithServiceWorker\wwwroot\js\site.js - code loading sw.js

NoHotReloadWithServiceWorker\Pages\Shared_Layout.cshtml - place where <script src="/_framework/aspnetcore-browser-refresh.js"></script> is injected manually NoHotReloadWithServiceWorker\Pages\Index.cshtml - page which aftter modification should auto-refresh in SignalR-connected browser

Exceptions (if any)

No response

.NET Version

6.0.101

Anything else?

VS2022 - Professional Version 17.0.5

$ dotnet --info
Zestaw .NET SDK (odzwierciedlenie dowolnego pliku global.json):
 Version:   6.0.101
 Commit:    ef49f6213a

Środowisko uruchomieniowe:
 OS Name:     Windows
 OS Version:  10.0.19043
 OS Platform: Windows
 RID:         win10-x64
 Base Path:   C:\Program Files\dotnet\sdk\6.0.101\

Host (useful for support):
  Version: 6.0.1
  Commit:  3a25a7f1cc

.NET SDKs installed:
  5.0.404 [C:\Program Files\dotnet\sdk]
  6.0.101 [C:\Program Files\dotnet\sdk]

.NET runtimes installed:
  Microsoft.AspNetCore.All 2.1.30 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.All]
  Microsoft.AspNetCore.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.AspNetCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.AspNetCore.App]
  Microsoft.NETCore.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.NETCore.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.NETCore.App]
  Microsoft.WindowsDesktop.App 3.1.22 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 5.0.13 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]
  Microsoft.WindowsDesktop.App 6.0.1 [C:\Program Files\dotnet\shared\Microsoft.WindowsDesktop.App]

To install additional .NET runtimes or SDKs:
  https://aka.ms/dotnet-download

[ChaosEngine]

I think I've found root cause: here Service Worker served requests have sec-fetch-dest: empty while "normal-requests" are sec-fetch-dest: document.

[mkArtakMSFT]

Thanks for contacting us, @ChaosEngine. Can you try to simplify the service worker code to no-op (no logic almost) and then confirm that the behavior persists with that too?

[ghost]

Hi @ChaosEngine. We have added the "Needs: Author Feedback" label to this issue, which indicates that we have an open question for you before we can take further action. This issue will be closed automatically in 7 days if we do not hear back from you by then - please feel free to re-open it if you come back to this issue after that time.

[SteveSandersonMS]

BTW the minimal service worker file would look something like just this:

self.addEventListener('fetch', () => { });

If you could let us know if things work with that service worker (remember to delete all state for your site from the browser to make sure it takes effect) then we'll have more insight into whether this is specific to your service worker logic. Thanks!

[ChaosEngine]

BTW the minimal service worker file would look something like just this:

Thank you for quick reaction.

self.addEventListener('fetch', () => { });

If you could let us know if things work with that service worker (remember to delete all state for your site from the browser to make sure it takes effect) then we'll have more insight into whether this is specific to your service worker logic. Thanks!

This minimalist and simplistic approach from above will work properly because it does not use fetch underneath. It is the use of fetch() as indicated here or here that is changing sec-fetch-dest to empty which in turn blocks emitting of script inclusion.

And most usage of service workers is with fetch() and caching it's responses. PS. I simplified the sw.js in repro project to be more readable yet still use fetch(0 and repro the issue

[SteveSandersonMS]

Thanks @ChaosEngine, that clarifies things a lot.

So it seems like a valid fix would be to change https://github.com/dotnet/sdk/blob/14b117b7088653b694e16ac2071fcbf634a2a9ab/src/BuiltInTools/BrowserRefresh/BrowserRefreshMiddleware.cs#L71 to explicitly allow Sec-Fetch-Dest: empty. Or even change it so it explicitly blocks iframe but leaves everything else. Do you agree?

@pranavkm Do you know if there was any reason we couldn't allow everything except iframe?

[pranavkm]

Do you know if there was any reason we couldn't allow everything except iframe?

Some of the destinations such as images, videos, audio etc seem like the wrong place to be injecting scripts in to. Somebody also brought up a bunch of other scenarios that might not be appropriate, so I'd prefer if we started conservatively.

[SteveSandersonMS]

Some of the destinations such as images, videos, audio etc seem like the wrong place to be injecting scripts in to

We might be able to filter on accept: text/html to avoid those, but I take your point that moving conservatively is wise.

[ChaosEngine]

Thanks @ChaosEngine, that clarifies things a lot.

So it seems like a valid fix would be to change https://github.com/dotnet/sdk/blob/14b117b7088653b694e16ac2071fcbf634a2a9ab/src/BuiltInTools/BrowserRefresh/BrowserRefreshMiddleware.cs#L71 to explicitly allow Sec-Fetch-Dest: empty. Or even change it so it explicitly blocks iframe but leaves everything else. Do you agree?

@pranavkm Do you know if there was any reason we couldn't allow everything except iframe?

Yes, or at least mention it on docs page to be searchable. In the end, manually adding script for debug build is no big of a deal.

Also I did a <iframe /> test including page content with service worker operating inside and it was also "rewritten" do Sec-Fetch-Dest: empty. Without SW it is set to Sec-Fetch-Dest: iframe. IMVHO this could an edge case.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 7 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[ghost]

We've moved this issue to the Backlog milestone. This means that it is not going to be worked on for the coming release. We will reassess the backlog following the current release and consider this item at that time. To learn more about our issue management process and to have better expectation regarding different types of issues you can read our Triage Process.

[ghost]

Thanks for contacting us.

We're moving this issue to the .NET 9 Planning milestone for future evaluation / consideration. We would like to keep this around to collect more feedback, which can help us with prioritizing this work. We will re-evaluate this issue, during our next planning meeting(s). If we later determine, that the issue has no community involvement, or it's very rare and low-impact issue, we will close it - so that the team can focus on more important and high impact issues. To learn more about what to expect next and how this issue will be handled you can read more about our triage process here.

[mkArtakMSFT]

Moving to .NET 9. We think this will be automatically solved when we fix #45213