Skip to content
This repository was archived by the owner on May 22, 2021. It is now read-only.

Show error page if upload fails#168

Merged
dannycoates merged 2 commits intomasterfrom
ui
Jul 10, 2017
Merged

Show error page if upload fails#168
dannycoates merged 2 commits intomasterfrom
ui

Conversation

@dnarcese
Copy link
Copy Markdown
Contributor

@dnarcese dnarcese commented Jul 6, 2017

When a file cannot be uploaded, show a very simple error page (which will be changed with new designs).

@dnarcese dnarcese requested a review from ericawright July 6, 2017 21:19
Copy link
Copy Markdown
Contributor

@ericawright ericawright left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good for now.
I think eventually we want to be able to find the reason for the failure and show that to the user on the error page

resolve(new Uint8Array(this.result));
};
reader.onerror = function(event) {
reject(event.explicitOriginalTarget.error.name);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dnarcese @abhinadduri Are these errors logged and sent through Raven?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nope, should be fairly easy to send them to Raven though.

notify('Your upload has finished.');
})
.catch(err => {
console.log('Upload error name: ' + err);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above, are these errors logged and sent through Raven at all?

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is a bit more tricky: if the user accidentally tampers with the salt, this error would trigger. Is that something we would want to send to Raven?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that only sending the error to Raven here is sufficient? This will eliminate any duplicates, since errors from the reader.onerror triggers (#168 (diff)) are always caught here.

@dannycoates dannycoates merged commit 125e6ec into master Jul 10, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants