dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.22k stars 9.95k forks source link

Safari - Unhandled Promise Rejection - NotifyChange #38657

Closed sipi41 closed 2 years ago

sipi41 commented 2 years ago

Eureka! I would like to ask you for help with a problem I'm facing. I created a form (for my daughter's dog grooming website) to input new pets, and it works on desktop pc's, all browsers (chrome, firefox, edge) and mobiles (android). THE PROBLEM comes when we try to run our app in iOS (any version of iPad or iPhone).

WHAT FAILS? In the form I do ask for a picture of the pet, when they select a file I ask a JS method to try to read the file, if this is a valid image, then it reads the image and using canvas compress and resize the image (you are maybe asking why not doing this in blazor, as we can read the file there and resize, etc... long story short, after resize, the resulting image size is way huge than if we do it using JS and also the base64 string produced damaged images, I did notify about that in other post but that's another story) continuing then the base64 string with the reduced image is returned where we save it at our model and we have a second field (as flag) that we change to true, meaning a valid image has been provided, please check the following screenshot:

error blazor server2

And here's a screenshot of the method fired when the file field changes:

UploadFileHandler

And here's a simple explanation of what this do:

  1. I invoke a javascript method passing a reference to the input field, javascript may or may not send a string, as this depends if the file selected is a valid image or not.
  2. If a valid base64 string result is received, we write this into the field in our model that will hold the image (in this case, ProfileImage)
  3. The last 2 lines create a FieldIdentifier reference and call the FormContext method to notify it was a change, in other words, the field that holds the image is marked as Required, so if the field is empty it will throw an error, but if a new image is received, we write this data then we call for the validation to be fired on this field, saying this field has changed, may or may not contain the requested value, that's up to the validator to decide.

What happened is that we were so happy to have this ready when my wife decided to test with an iPhone, all validations, events, etc did work except this one, no ugly errors on safari, just the image did not appear. I found how difficult is to a windows developer test on an apple computer unless you have one at home, which is not my case, emulators? no, none of them work, the one included with VS works only with MAUI (+ $90x subscription fee) which I'm not sure what it is... so I finally ended building a fake system with VirtualBox after several tries (to download the right version and configure this thing), and here's an screenshot of what's going on:

error blazor server

I'm not sure what this error means exactly or how to fix it but it is something produced at the blazor.js script. It would be fantastic if you could provide us with some idea of what's going on, thank you so much for any help you may bring to this situation.

I can share my project if you would like to inspect it a little more, what I can't give is my database server user/pass but I'm sure you could create your own db to test, as all code is there but not sure what you may need, thank you.

Here's another look to the error, complete: image

aloksharma1 commented 2 years ago

why not simply use image compression & resize middleware like these ImageSharp

or using skia

sipi41 commented 2 years ago

@aloksharma1 Hi there!... what happens is, when you use the proposed solutions, you have to transfer the whole image to the server to be resized there, the idea is to avoid huge transfers (images can be 10Mb even more), so instead of transferring the file as is to the server, we resize using JS and the final product is the one transferred... but this has nothing to do with the problem we are facing with Safari, that's another story.

aloksharma1 commented 2 years ago

@sipi41 you can always delete original image after post processing data (shrink, resize do whatever then delete original & keep copy), anyways this seems like a issue with js breaking on function call, which unfortunately cant be solved without live debugging. Your error result seems like from production, can you try from a debugger; most probably its happening with your js invoke (try put some console.logs in func calls). maybe microsoft guys will have more ideas, but my solution will also work as i am using it in my projects.

sipi41 commented 2 years ago

@aloksharma1 No, the error is not on my JS interop, as you can see the error is reported to be in blazor.server.js not in my code, otherwise the signalR call breaks, not the case here, where did you get that? in fact, I think the problem comes into place when the EditContext.NotifyFieldChanged is invoked, but that's not related to JS...

mkArtakMSFT commented 2 years ago

Thanks for contacting us. We've improved this area in .NET 6 and here are some docs to help you get started. Can you please check it out and see whether the new changes in .NET 6 resolve your issue: https://docs.microsoft.com/en-us/aspnet/core/blazor/images?view=aspnetcore-6.0

ghost commented 2 years ago

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

sipi41 commented 2 years ago

thank you @mkArtakMSFT unfortunately I think there's a misunderstanding, because the problem is not related to images, the problem is this: blazor.server.js is throwing an ugly error when invoking NotifyFieldChanged method, which is after the image process... is not my javascript that is generating this problem but something inside blazor.server.js.

I did offer to send you a copy of my project for you to test on a safari / macos computer, I repeat, it's not about image, or javascript, its something wrong when using Form Context and NotifyFieldChanged

TanayParikh commented 2 years ago

then it reads the image and using canvas compress and resize the image (you are maybe asking why not doing this in blazor, as we can read the file there and resize, etc... long story short, after resize, the resulting image size is way huge than if we do it using JS and also the base64 string produced damaged images, I did notify about that in other post but that's another story) continuing then the base64 string with the reduced image is returned where we save it at our model and we have a second field (as flag) that we change to true, meaning a valid image has been provided, please check the following screenshot:

what happens is, when you use the proposed solutions, you have to transfer the whole image to the server to be resized there, the idea is to avoid huge transfers (images can be 10Mb even more), so instead of transferring the file as is to the server, we resize using JS and the final product is the one transferred

As Artak mentioned above, we recommend upgrading to .NET 6 in this case as there have been extensive improvements made in the image handling capabilities. The new Blazor image handling doesn't require Base64 at all (see https://docs.microsoft.com/en-us/aspnet/core/blazor/images?view=aspnetcore-6.0). Doing so will allow you to avoid spinning up your own image handling mechanics, and may help resolve the NotifyChange issue as well. Note, to resize an image in Blazor, a file reference is sent to / from client and server (not the full image). Blazor still does resizing in the client using JS.

unfortunately I think there's a misunderstanding, because the problem is not related to images, the problem is this: blazor.server.js is throwing an ugly error when invoking NotifyFieldChanged method, which is after the image process... is not my javascript that is generating this problem but something inside blazor.server.js.

Taking the recommended steps above (.NET 6 and using Blazor's image handling support) will likely resolve this issue. If you're still having this issue, please provide a minimal public github repository which reproduces this issue (without requiring a database).

ghost commented 2 years ago

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

sipi41 commented 2 years ago

@TanayParikh thank you!

Regarding the image problem (REMEMBER, THIS IS NOT ABOUT IMAGE UPLOAD/RESIZING BUT A PROBLEM WITH SAFARI, HOW I TREAT IMAGES IS ANOTHER STORY, NOT RELATED TO THIS POST), yes, I did not exactly as docs say, as the docs say we should pass the image stream to the client via JS, these are the approaches I did take as my understanding is that EF can't save blob images into the db, so we must have a URL or a base64 string to save the image, (am I missing something?):

Acording to the Docs I should use the streaming RequestImageFileAsync to resize the image, then save it to a file or anything else, and it works perfectly except when:

  1. You want to compress the file... for example using RequestImageFileAsync resize the image but the result is for example: a file of 250kb is resized to 210kb... using JS canvas, the same file can be reduced to 80kb, same size, same quality.
  2. can I use ImageSharp? sure, but this implies that I have to transfer a 10Mb image to the server to be manipulated, not a good way to go...

So this is what I did... I did pass InputFile reference to a JS method, where I try to read the image, resize and return the desired base64 string to my method, where I pass this to an image and is saved to my DB. I think this is a valid approach, maybe not the one everybody recommends but it works.

What fails here is that I want to fire the validation over this field, before was empty but when a valid image string is received its filled with the result, so the validation is requested, there is when it fails... so is not related to the image, its related to invoking the NotifyChange

I will provide a sample of the project, the only thing is, this problem is happening only on safari browser.

TanayParikh commented 2 years ago

are the approaches I did take as my understanding is that EF can't save blob images into the db, so we must have a URL or a base64 string to save the image, (am I missing something?):

Is it a requirement to save images to DB? Can't they just be stored as files on disk & read from/written to using file streaming?

I will provide a sample of the project, the only thing is, this problem is happening only on safari browser.

Just to re-iterate above, please ensure it is a minimal, public github repro application. I'm able to test on Safari so that's not an issue 😄. Also please ensure this is a .NET 6 application.

ghost commented 2 years ago

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

sipi41 commented 2 years ago

@TanayParikh Thank you again for the quick follow-up. Yes, its a requirement, they don't want to have files somewhere then the db records somewhere else, and these are 2 pictures only, before and after picture (these are dogs, before cleaning the dog, after cleaning the dog)

I will upload the project as soon as I finish something (as I'm working now), not sure what do you mean by minimal, but its a small project.

TanayParikh commented 2 years ago

not sure what do you mean by minimal, but its a small project.

The repro should just contain the minimum amount of code to trigger the exception you're seeing. No extra application logic, assets, or dependencies. I recommend doing dotnet new blazorwasm or dotnet new blazorserver and adding in the breaking code into the Index.razor file. This ensures we're able to quickly analyze and diagnose the issue at hand.

ghost commented 2 years ago

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

sipi41 commented 2 years ago

@TanayParikh ok, I will do my best :-)

sipi41 commented 2 years ago

@TanayParikh here's the link to the repository: https://github.com/sipi41/HappyDogsAppSample

I tried by best to reduce it, I hope this can clarify more what is causing this issue.

I tried also to run this on a apple virtual machine, I installed VS and created a sample project (a console app), it did work... then a msg of an update, I did it and then the app I created before, stoped working... tried to open the project I uploaded and also fails, at the moment of pressing F6 check it out... so I was unable to test on mac by myself, I hope that will not be the case for you...

PLEASE test this first in windows, and you will see it works as expected... basically you enter the name of the pet, then choose a picture... please let me know what did you find out. Thanks for all your help.

ErrorMac .

gulbanana commented 2 years ago

I have a Windows PC and a Macbook, I will test your repro on both.

gulbanana commented 2 years ago

As expected, it works when using Edge on Windows: https://i.imgur.com/WtJHedE.png

gulbanana commented 2 years ago

When using Safari on MacOS, I can reproduce the problem: https://i.imgur.com/SKJsHzd.png

gulbanana commented 2 years ago

I've found the bug - I'm afraid it's in your code, not in Blazor. In AddNewPetForm.razor.js, you have a function which begins like this:

    ReadAndReduceImage: async function(InputFileRef) {

        console.clear();

        var InputFileRef = InputFileRef.element;

        var Files = InputFileRef.files;

"var InputFileRef = InputFileRef.element;" is not correct. var is used in javascript to declare a variable for the first time, not to update it; additionally, if you use var (rather than let or const) then the variable declaration is "hoisted", meaning it's moved to the start of the function.

How exactly this interacts with the variable being a parameter is unspecified, but Safari's behaviour is legit here - it assumes that you are referring to the local variable InputFileRef, setting it to its own .element property, before it has been defined. Hence the error, undefined is not an object [and therefore it cannot read the property element].

You can fix the bug by changing var InputFileRef = InputFileRef.element; to InputFileRef = InputFileRef.element;.

sipi41 commented 2 years ago

@gulbanana thanks for the feedback... I'm still stuck with VS in the macos virtual machine, after an update it simply doesn't work...

wow... this surprise me, as this is not the original error (as you can see on top), ok...

First question... after your proposed modifications does it work as expected?

Do you propose to use let? or const instead? this is a very obscure "thing" as most of things in apple...

TanayParikh commented 2 years ago

Thanks @gulbanana for looking into this!

wow... this surprise me, as this is not the original error (as you can see on top), ok...

Your original screenshot / error was in .NET 5 I believe. This repro app is in .NET 6 which may explain the slightly different error.

Do you propose to use let? or const instead?

You don't need either in this case as you can just modify the parameter (InputFileRef) value. Alternatively, you can use const to ensure you're not changing the input param to the method:

    ReadAndReduceImage: async function(InputFileRef) {

        console.clear();

        const inputFileRefElement = InputFileRef.element;

        var Files = inputFileRefElement.files;
sipi41 commented 2 years ago

@gulbanana and @TanayParikh yes, thank you for your feedback... I did take a second look at my code and yes, the problem was not on using var or let, the problem was that I was calling my variable (the first one) the same name of the parameter received by the function, which caused the browser to throw an error.

I have corrected this error on my code, I will re-publish the original project and see if it works, as the first error was not resolved and/or discovered (view first post).

sipi41 commented 2 years ago

FINAL CONCLUSION: Yes, the error is resolved, the first error never appeared and it works as expected, did a test on iPhone and macOS.

It would be nice if we could suggest a modification to the IntelliSense of VS related to JS, when trying to create a variable with the same name of any of the parameters, throw an error because, this one was very hard to see, and even harder if most browsers (PC and Android) say everything is ok...

Is there any plug-in that can be installed on VS IDE that could help with JS? as you may know, many of us are c# developers and are not really experts on javascript, some extra help could be so great!

Anything you may want to add? again, thank you for your support and patience.

TanayParikh commented 2 years ago

Hi @sipi41, glad to see this is resolved.

It would be nice if we could suggest a modification to the IntelliSense of VS related to JS, when trying to create a variable with the same name of any of the parameters, throw an error because, this one was very hard to see, and even harder if most browsers (PC and Android) say everything is ok...

Feel free to Report a Problem in VS and provide this feedback / request this analyzer / code fix for JS / Razor.