-
Notifications
You must be signed in to change notification settings - Fork 1.5k
Conversation
There was a problem hiding this 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
frontend/src/fileSender.js
Outdated
@@ -51,6 +51,9 @@ class FileSender extends EventEmitter { | |||
reader.onload = function(event) { | |||
resolve(new Uint8Array(this.result)); | |||
}; | |||
reader.onerror = function(event) { | |||
reject(event.explicitOriginalTarget.error.name); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
frontend/src/upload.js
Outdated
notify('Your upload has finished.'); | ||
}) | ||
.catch(err => { | ||
console.log('Upload error name: ' + err); |
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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?
There was a problem hiding this comment.
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.
When a file cannot be uploaded, show a very simple error page (which will be changed with new designs).