dotnet / runtime

.NET is a cross-platform runtime for cloud, mobile, desktop, and IoT apps.
https://docs.microsoft.com/dotnet/core/
MIT License
14.53k stars 4.53k forks source link

[blazor-wasm] Improve handling OOM error, and surface it better #95971

Open radical opened 2 years ago

radical commented 2 years ago

Example issue: https://github.com/dotnet/aspnetcore/issues/40528

If the initial heap size is too big (set via $(EmccTotalMemory)) then the user sees:

_framework/blazor.boot.json
blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 Streaming compilation failed. Falling back to ArrayBuffer instantiation.  RangeError: WebAssembly.instantiate(): Out of memory: wasm memory
blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 TypeError: Failed to execute 'arrayBuffer' on 'Response': body stream already read
window.Module.s.printErr @ blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1
(anonymous) @ blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1
await in (anonymous) (async)
window.Module.s.instantiateWasm @ blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1
createWasm @ dotnet.6.0.2-mauipre.1.22102.15.0nf0a6txvs.js?v=sha256-/5MtncfJpmo16luI9orAGCmbYEhUFE8G+iO2w9+y4x8=:1
(anonymous) @ dotnet.6.0.2-mauipre.1.22102.15.0nf0a6txvs.js?v=sha256-/5MtncfJpmo16luI9orAGCmbYEhUFE8G+iO2w9+y4x8=:1
blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 Uncaught (in promise) TypeError: Failed to execute 'arrayBuffer' on 'Response': body stream already read
    at blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1:36928
    at async blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1:36900
    at async blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1:36636

We should instead catch the OOM error, and maybe surface it as better error that the app can catch, and surface to the user. And log a useful error message on how to fix it, for the the app developer. And with https://github.com/dotnet/aspnetcore/issues/40547 , it won't make an unnecessary second attempt.

@kg @lewing @pranavkm

javiercn commented 2 years ago

@radical this seems like a good improvement.

Do you have any concrete suggestion on the error message, etc?

ghost commented 2 years ago

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.

mkArtakMSFT commented 2 years ago

@radical can you please clarify what your suggestion here is? Also, just double checking whether you meant that the initial heap size is indeed too big or small instead?

radical commented 2 years ago

Can you please clarify what your suggestion here is?

I'm suggesting that we should catch this RangeError, and if it is "Out of memory: wasm memory", then:

  1. don't fallback;
  2. maybe raise an error like OutOfMemoryError, which the app can catch, and inform it's user accordingly;
  3. And log an error saying that the device doesn't have sufficient heap memory = value, and suggesting that they can change the initial heap memory size by setting the $(EmccInitialHeapSize)[1] property in their project file.

And fix https://github.com/dotnet/aspnetcore/issues/40547 .

All this would then avoid cryptic errors for which the solutions ends up being - change $(EmccInitialHeapSize) for your device

--

  1. $(EmccTotalMemory) was renamed to $(EmccInitialHeapSize) today. But the original property will continue to work.
CoryKoehler commented 2 years ago

I am cross posting this as it may be helpful from https://github.com/dotnet/aspnetcore/issues/40547

"Hello, I put in an issue similar and closed it since this one seems to be the one where everything is tracked regarding this. I don't think this can be pushed to .Net7. I am having customers complain currently on this. Depending on device (both Android and IOS are affected)

This to me seems huge and would make my clients rethink any investment in Blazor. An existing production site that is blazor wasm hosted that is having this issue is boatsforsale.com and replicate this issue with enough attempts on smaller devices. The site will display just a loading spinner forever and in the console you'll see image

Is there any way this could be looked into for an earlier release than next nov? @chassq "

--Edit below I see this is triaged and Prio1. Does this mean it will go sooner than nov? If so any idea on it? I do need to communicate this asap to my clients.

CoryKoehler commented 2 years ago

Also would setting<EmccTotalMemory>16777216</EmccTotalMemory> fix this for users of android/ios ? or do I need to set <EmccInitialHeap>16777216</EmccInitialHeap> What are the default values for this? Would I be breaking users by increasing this value? is there any documented guidance on this that I can read?

lewing commented 2 years ago

if you are using .NET 6 use EmccTotalMemory (it will continue to work), the rename is around clarity.

CoryKoehler commented 2 years ago

What happens if I choose 16gb and someone on an phone doesn't have 16gb of memory? What are the error cases with this?

kg commented 2 years ago

WASM is 32-bit right now so anything above 4gb definitely won't work, and in some cases 2gb is your hard limit, FYI.

CoryKoehler commented 2 years ago

WASM is 32-bit right now so anything above 4gb definitely won't work, and in some cases 2gb is your hard limit, FYI.

ok that is good to know, I just don't want to apply this change/fix to my production system and then be crashing peoples devices

kg commented 2 years ago

there is still a soft limit where your device or browser may not be able to allocate the requested amount of memory, and that soft limit is likely lower than 2gb on constrained devices like mobile handsets. in some cases instead of failing to allocate the memory the browser tab will go away because the mobile approach to memory management is to randomly kill processes when memory is low.

CoryKoehler commented 2 years ago

there is still a soft limit where your device or browser may not be able to allocate the requested amount of memory, and that soft limit is likely lower than 2gb on constrained devices like mobile handsets. in some cases instead of failing to allocate the memory the browser tab will go away because the mobile approach to memory management is to randomly kill processes when memory is low.

Ah I wish it was killing it when the error I mentioned further up happens. That would be a bit better than a forever spinner. But I will try this since it can't be any worse I suppose. @todd-urbo

ghost commented 8 months ago

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 commented 6 months ago

@radical, @lewing this doesn't seem something that should be fixed in the Blazor layer, rather on the runtime (if at all). So I'm going to move this to the runtime repo for you to have a look.

ghost commented 6 months ago

Tagging subscribers to 'arch-wasm': @lewing See info in area-owners.md if you want to be subscribed.

Issue Details
Example issue: https://github.com/dotnet/aspnetcore/issues/40528 If the initial heap size is too big (set via `$(EmccTotalMemory)`) then the user sees: ``` _framework/blazor.boot.json blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 Streaming compilation failed. Falling back to ArrayBuffer instantiation. RangeError: WebAssembly.instantiate(): Out of memory: wasm memory blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 TypeError: Failed to execute 'arrayBuffer' on 'Response': body stream already read window.Module.s.printErr @ blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 (anonymous) @ blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 await in (anonymous) (async) window.Module.s.instantiateWasm @ blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 createWasm @ dotnet.6.0.2-mauipre.1.22102.15.0nf0a6txvs.js?v=sha256-/5MtncfJpmo16luI9orAGCmbYEhUFE8G+iO2w9+y4x8=:1 (anonymous) @ dotnet.6.0.2-mauipre.1.22102.15.0nf0a6txvs.js?v=sha256-/5MtncfJpmo16luI9orAGCmbYEhUFE8G+iO2w9+y4x8=:1 blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1 Uncaught (in promise) TypeError: Failed to execute 'arrayBuffer' on 'Response': body stream already read at blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1:36928 at async blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1:36900 at async blazor.webassembly.js?v=pD9dkEr0gbzoZKvepQmzbYKQYka7z-K9H2I-m5Inpxc:1:36636 ``` We should instead catch the OOM error, and maybe surface it as better error that the app can catch, and surface to the user. And log a useful error message on how to fix it, for the the app developer. And with https://github.com/dotnet/aspnetcore/issues/40547 , it won't make an unnecessary second attempt. @kg @lewing @pranavkm
Author: radical
Assignees: -
Labels: `arch-wasm`, `untriaged`, `area-VM-meta-mono`, `needs-area-label`
Milestone: -