Skip to content

refactor(crypto): minor refactor of decryptWithAttachments#65

Merged
seaerchin merged 13 commits intodevelopfrom
refactor/crypto
Jun 2, 2021
Merged

refactor(crypto): minor refactor of decryptWithAttachments#65
seaerchin merged 13 commits intodevelopfrom
refactor/crypto

Conversation

@seaerchin
Copy link
Contributor

Problem

Refactored decryptWithAttachments for additional clarity

Issues faced

  1. lack of chaining means that everything has to be within a try/catch block

@seaerchin seaerchin requested a review from mantariksh May 17, 2021 04:33
@seaerchin seaerchin requested a review from mantariksh May 17, 2021 08:37
@seaerchin seaerchin requested review from mantariksh and removed request for mantariksh May 20, 2021 02:36
@seaerchin seaerchin requested a review from mantariksh May 27, 2021 06:53
class AttachmentDecryptionError extends Error {
constructor(message = 'Attachment decryption with the given nonce failed.') {
super(message)
this.name = this.constructor.name
Copy link
Contributor

Choose a reason for hiding this comment

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

out of curiosity, why is this line necessary?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

it isn't! i actually left it in because i thought it was convention @__@ i'll remove it!

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i went to investigate more and it was q enlightening for me! the reason why this is done is because it's an error. when js (and by extension, ts) throws an error, the error printed to the output is done using error.name + ' ' + error.message - this means that when the error here is thrown, because it's missing the name property, it will default to Error (as it calls super(message)). While the argument might be that it's guaranteed to never be thrown, i think this is a relatively small change that helps in the future so i'd just leave it in!

@seaerchin seaerchin merged commit 884cd83 into develop Jun 2, 2021
@seaerchin seaerchin deleted the refactor/crypto branch June 2, 2021 03:03
@seaerchin seaerchin restored the refactor/crypto branch June 2, 2021 03:11
@mantariksh mantariksh deleted the refactor/crypto branch June 17, 2021 04:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants