1295970 - Error handling when cloud file upload fails not optimal, no error shown. Also needs to cover case where event is closed before upload finishes.

Status: NEW

(Calendar :: Dialogs, defect, P5)

Description

[Jorg K (CEST = GMT+2)]

As per bug 1295525 comment #9 (without the grammar errors):

I've tried with varying success. I got three cases:
1) File uploaded but link was to local file in attachment box.
2) File uploaded but no link was inserted.
3) Success: File uploaded and link placed into attachment box.

I've also seen heaps of warnings on the JS console:
TypeError: parent is null lightning-item-iframe.js:156:5
[calendar-event-dialog] Uploading cloud attachment failed. Status code: 2153066778 lightning-item-iframe.js:2066:17
NS_ERROR_NOT_INITIALIZED:

Looks like these were caused by the problems mentioned in 1) and 2). I've created some more events with cloud attachments and now I saw consistent success with no messages on the error console.

I also saved and closed the event before the upload was finished, that might have caused the "parent is null" error.

Note that were the upload failed, I got no error message. I just failed silently.

Comment 1

[Paul Morris [:pmorris]]

Thanks for testing and filing a new bug.  I tried uploading attachments while working on the previous bug 1295525 (with my brand new box.com account).  I got a mix of successes and failures, but did not look closer, due to focusing on the other issue.  

Trying it again now, I find that the first time I attach/upload a given file it succeeds, but if I try with the same file again it fails.

The failures I see give the following in the error console:
[calendar-event-dialog] Uploading cloud attachment failed. Status code: 2153066778lightning-item-iframe.js:2066:17

And I see a 5 second [!] icon next to the name of the file in the attachments tab, before it disappears.  As far as I can see that is the correct current way to report errors with uploading.  (There's a code comment about using a notification bar in the future.)  Looking over the code around where the error arises I haven't found anything that seems related to the event in a tab changes, so I'm not sure what's going on.

Comment 2

[Jorg K (CEST = GMT+2)]

What about the:
TypeError: parent is null lightning-item-iframe.js:156:5
That's yours, right?

Have you tried closing the event while the upload is going?

Comment 3

[Paul Morris [:pmorris]]

I was able to reproduce this error:
    TypeError: parent is null lightning-item-iframe.js:156:5
by closing the tab/window while the upload is happening.  It seems the setTimeout for a failed upload (on line 2080) fires after the tab/window has closed and parent is null.

I was able to prevent it by adding an "if (parent) {...}" conditional, and will upload a patch for that at least.

Comment 4

[Paul Morris [:pmorris]]

Attachment: parent-is-null-fix.patch

Prevents 'parent is null' error in the console.

Comment 5

[[:MakeMyDay]]

Comment on attachment 8782259 [details] [diff] [review]
parent-is-null-fix.patch

Review of attachment 8782259 [details] [diff] [review]:
-----------------------------------------------------------------

r- for this approach. What needs to be done here is to track whether there's and upload in progress and prevent closing until it is completed. In case of a failure, the user should be notified accordingly. This can be done using the notification bar, which is already implemented. Maybe it makes sense to use the bar also to indicate the upload in progress.

But this use case must have failed already before the tab implementation, e.g. in the current ESR version 4.7.x either with or without a message in the log.

Comment 6

[Jorg K (CEST = GMT+2)]

(In reply to [:MakeMyDay] from comment #5)
> What needs to be done here is to track whether there's
> and upload in progress and prevent closing until it is completed.
How does the user cancel an upload that is taking too long or gets hung for some reason?
Oh, I see, right click on the attachment during the upload and select "Remove". 

> In case of a failure, the user should be notified accordingly.
Agreed.

> Maybe it makes sense to
> use the bar also to indicate the upload in progress.
Well, there is a little spinning blue circle in this case.

What about
NS_ERROR_NOT_INITIALIZED: lightning-item-iframe.js:2078
Sorry, I didn't report the line number before.

Comment 7

[Jorg K (CEST = GMT+2)]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #6)
> How does the user cancel an upload that is taking too long or gets hung for
> some reason?
> Oh, I see, right click on the attachment during the upload and select
> "Remove". 
Oh, looks like you cannot cancel an upload. It always runs to completion regardless of whether you close the tab/window or "Remove". Hmm.

Comment 8

[Paul Morris [:pmorris]]

(In reply to Jorg K (GMT+2, PTO during summer) from comment #6)
> What about
> NS_ERROR_NOT_INITIALIZED: lightning-item-iframe.js:2078

This is where the setTimeout is called to show the failed upload icon for 5 seconds before removing the attachment from the list.  I don't get this error until I close a tab before the file is done loading, and then I seem to get it every time after that regardless of whether I've closed the tab or not.

I think it might be better to avoid the setTimeout and immediately remove the attachment from the list, indicate the failure in a notification bar, and then let the user dismiss the notification bar.

...especially if that is where upload progress is indicated, as suggested above.  Another argument for putting the upload progress in a notification bar is that when the attachments tab is not activated the user can't see the progress or failure indications.

Comment 9

[[:MakeMyDay]]

This bug missed the cut for string changes, so I'm removing the tracking for 5.4.