Refactor WebCrypto to be tread safe: part 1 #75
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Merged
olegbespalov
changed the title
olegbespalov
requested review from
mstoykov and
oleiade
and removed request for
a team
olegbespalov
force-pushed
the
refactor/tread-safe
branch
from
acca5b4
to
9ad6664
Compare
mstoykov
reviewed
Apr 17, 2024
mstoykov
approved these changes
Apr 17, 2024
olegbespalov
force-pushed
the
refactor/tread-safe
branch
from
9ad6664
to
d3fa3aa
Compare
mstoykov
approved these changes
Apr 17, 2024
oleiade
reviewed
Apr 17, 2024
| reject(err) | ||
| return promise | ||
| } | ||
| err := func() error { |
There was a problem hiding this comment.
I'm not familiar with this pattern of wrapping erroring code in dedicated anonymous' function scope. Just informally, I'd be keen to hear more about its benefits 🙇🏻
There was a problem hiding this comment.
Sure 👍
The key point of this change is to centralize the logic of management promises. Previously, it was easier to miss that we either had to reject or return promises.
See examples:
- JWK Implementation for import/export #61 (comment)
- JWK Implementation for import/export #61 (comment)
Also, it simplifies the operations because at this logical block, we only do operations + with returning almost vanilla errors, but it doesn't do any promise-related operations.
oleiade
approved these changes
Apr 17, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What?
This change centralizes and unifies logic and ensures that runtime (
tr) isn't used outside the event loop.I also removed references to WebCrypto steps, like (
1.,2., etc.) since the issue is that for different algorithms, they could mean different steps, which could confuse future maintainers.Splitting the PR into the parts to make it smaller. #76
Why?
Updates #71