-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Conversation
frontend/src/fileSender.js
Outdated
@@ -54,7 +48,8 @@ class FileSender extends EventEmitter { | |||
['encrypt', 'decrypt'] | |||
) | |||
.catch(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.
we should just remove this catch
console.log('The file was successfully deleted.'); | ||
} else { | ||
console.log('The file has expired, or has already been deleted.'); | ||
} |
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 felt weird to kill, but wasn't really "adding" anything to the user experience.
Not sure if we should be doing some metrics logging here instead of just vanilla console.log()
.
frontend/src/fileSender.js
Outdated
@@ -54,7 +48,8 @@ class FileSender extends EventEmitter { | |||
['encrypt', 'decrypt'] | |||
) | |||
.catch(err => | |||
console.log('There was an error generating a crypto key') | |||
// eslint-disable-next-line no-console | |||
console.error('There was an error generating a crypto key') |
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.
Kept this since removing the catch()
would alter the code paths. This catch()
is actually swallowing the error, IIUC. So if window.crypto.subtle.generateKey()
fails for whatever reason, we're intercepting the error and just ignoring it, allowing Promise.all()
to resolve with potentially an undefined
or null
secretKey
.
Or not, maybe I'm wrong. 🤷♀️
server/server.js
Outdated
@@ -33,6 +33,7 @@ function allLangs() { | |||
} | |||
|
|||
function prodLangs() { | |||
// eslint-disable-next-line security/detect-non-literal-require |
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.
Unrelated, but this was the only remaining ESLint warning.
Looking again, that could have been normalized out by potentially doing require('../package.json')
and letting Node.js resolve paths internally (like it does elsewhere for require('./utils')
.
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.
yeah, why the f did i do it like this? 🍌
Fixes #283