Improve compatibility reporting by handling synchronous exceptions #33
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
When an exception is thrown by the synchronous part of a Promise-returning function, that exception is (by design) not handled by any catch clauses associated with the call. Because of this, synchronous exceptions in cryptographic calls by the sample page can bubble up as script errors and cancel execution of the current and all remaining calls in a script block. As a result, compatibility information on some platforms (for example, Safari on iOS 9) is incomplete. This change improves things by wrapping all calls to cryptographic functions in a Promise context so the existing catch clause executes and displays information about the relevant failure. Subsequent tests in the same script block are now able to run and display their own pass/fail information. To minimize churn, this change wraps all cryptographic functions in a Promise context so each of the existing calls are (relatively) unchanged. (Platforms that do not support Promises do not benefit, but are no worse off.) An alternate implementation would be to introduce a Promise wrapper around each of the initial calls in a chain, but that causes more significant changes to the code and obfuscates the intent somewhat. Another alternate implementation would be to wrap each chain in a try/catch block, but that seems even more impactful/confusing. Configuration for each of the algorithms was moved to the top of the file because execution is now more asynchronous and it is possible for a configuration to be used before being initialized.
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.
See commit description (below) for details.
Here is a live sample of the proposed changes:
https://webcryptoexamples.azurewebsites.net/
Here is what Safari on iOS 9 looks like today without the change:
https://webcryptoexamples.azurewebsites.net/iOS9-Safari-Before.png
And here's what it looks like with the proposed change:
https://webcryptoexamples.azurewebsites.net/iOS9-Safari-After.png
Note, for example, that importKey/AES-KW/raw-key is identified as being supported (vs. blank).
Tested on:
Improvements seen on:
When an exception is thrown by the synchronous part of a Promise-returning function, that exception is (by design) not handled by any catch clauses associated with the call. Because of this, synchronous exceptions in cryptographic calls by the sample page can bubble up as script errors and cancel execution of the current and all remaining calls in a script block. As a result, compatibility information on some platforms (for example, Safari on iOS 9) is incomplete.
This change improves things by wrapping all calls to cryptographic functions in a Promise context so the existing catch clause executes and displays information about the relevant failure. Subsequent tests in the same script block are now able to run and display their own pass/fail information.
To minimize churn, this change wraps all cryptographic functions in a Promise context so each of the existing calls are (relatively) unchanged. (Platforms that do not support Promises do not benefit, but are no worse off.) An alternate implementation would be to introduce a Promise wrapper around each of the initial calls in a chain, but that causes more significant changes to the code and obfuscates the intent somewhat. Another alternate implementation would be to wrap each chain in a try/catch block, but that seems even more impactful/confusing. Configuration for each of the algorithms was moved to the top of the file because execution is now more asynchronous and it is possible for a configuration to be used before being initialized.