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

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

Closed
radical opened this issue Mar 4, 2022 · 17 comments
Closed

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

radical opened this issue Mar 4, 2022 · 17 comments
Assignees
Milestone

Comments

@radical
Copy link
Member

radical commented Mar 4, 2022

Example issue: dotnet/aspnetcore#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 dotnet/aspnetcore#40547 , it won't make an unnecessary second attempt.

@kg @lewing @pranavkm

@javiercn
Copy link
Member

javiercn commented Mar 7, 2022

@radical this seems like a good improvement.

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

@ghost
Copy link

ghost commented Mar 7, 2022

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
Copy link
Member

@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
Copy link
Member Author

radical commented Mar 8, 2022

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 dotnet/aspnetcore#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.

@mkArtakMSFT mkArtakMSFT added the Priority:1 Work that is critical for the release, but we could probably ship without label Mar 23, 2022
@CoryKoehler
Copy link

I am cross posting this as it may be helpful from dotnet/aspnetcore#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
Copy link

CoryKoehler commented Mar 25, 2022

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
Copy link
Member

lewing commented Mar 25, 2022

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

@CoryKoehler
Copy link

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
Copy link
Member

kg commented Mar 25, 2022

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
Copy link

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
Copy link
Member

kg commented Mar 25, 2022

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
Copy link

CoryKoehler commented Mar 25, 2022

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
Copy link

ghost commented Oct 6, 2023

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
Copy link
Member

@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.

@mkArtakMSFT mkArtakMSFT removed bug Priority:1 Work that is critical for the release, but we could probably ship without labels Dec 13, 2023
@dotnet-issue-labeler dotnet-issue-labeler bot added the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Dec 13, 2023
@ghost ghost added the untriaged New issue has not been triaged by the area owner label Dec 13, 2023
@mkArtakMSFT mkArtakMSFT transferred this issue from dotnet/aspnetcore Dec 13, 2023
@radical radical added arch-wasm WebAssembly architecture area-VM-meta-mono labels Dec 13, 2023
@ghost
Copy link

ghost commented Dec 13, 2023

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

Issue Details

Example issue: dotnet/aspnetcore#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 dotnet/aspnetcore#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: -

@vcsjones vcsjones removed the needs-area-label An area label is needed to ensure this gets routed to the appropriate area owners label Dec 18, 2023
@lewing lewing added this to the 9.0.0 milestone Apr 1, 2024
@lewing lewing removed the untriaged New issue has not been triaged by the area owner label Apr 1, 2024
@pavelsavara
Copy link
Member

RangeError: WebAssembly.instantiate(): Out of memory: wasm memory should be easy to catch here

We could translate that error to something which would be easier to understand for the developer.

We don't fallback in instantiation anymore. We do it in the compilation
But that should not fail on OOM.

@mkhamoyan
Copy link
Contributor

Error message was added by #104963

@github-actions github-actions bot locked and limited conversation to collaborators Aug 17, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

13 participants