Self-signed certificate - timeout error fix #6646
Merged
Conversation
This file contains hidden or 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
The `generateCertificate()` function had the try-catch result given only as the general failure message. To keep the user aware of issues during the RNW setup, this self-signed certificate generating try-catch block has been provided with the additional information included in the exception argument: err. Please note, that only the err is displayed, instead of extending it with additional parameters (such as ERRNO, or path) as these are developers details and it keeps the error message informative but still short.
The timeout set to 10 seconds when generating the self-signed certificate was causing the error on slower machines for developers. Please see: #6281 The idea was also to make this timeout value configurable, but in the end this solution would just make each developer leave it as default (magic number 10) or set it to max just so it's not needed to be set again. This could also result in trials and errors when trying to set the proper timeout value, which would make using the RNW uncomfortable.
ghost
added
the
|
|
Contributor
|
Giving to ACoates who added the timeout in #4430 |
asklar
approved these changes
Dec 2, 2020
Contributor
|
Just realized that ACoates is out of office. My sense is that the timeout isn't the right thing to do here, but from issue history it looks like a mitigation against unexplained hangs. Removing it seems fine to me, but would love to have a separate strategy for discovering root cause or mitigating these hangs in the wild. Better logging helps here, but I also wonder if it would make sense to add a CLI flag or fallback to temp keys for if customers do hit the issue. |
NickGerleman
approved these changes
Dec 2, 2020
dannyvv
approved these changes
Dec 2, 2020
Member
|
@BartoszKlonowski: Thank you for your contribution to react-native-windows. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
This pull request fixes #6281
Changes introduced in this PR provide the project with enhanced error printed when generating a self-signed certificate fails for any reason. The timeout set for generating the self-signed certificate is removed.
The implementation of this fix contains two main changes:
This allows any machine to generate the self-signed certificate no matter how slow it is (please see the linked issue).
Another idea was to make this timeout value configurable, but in the end it would just reduce the comfort of creating the RNW project, as the timeout would be one of the arguments, or it would be default
(leaving it as default would require changing the value, but to which value would be risky, as we never know how slow some developer's machine can be).
generateCertificate()function, by printing the originalerrmessage in additional to general dispatch saying that the generating failed.For more information about the implementation please see the commit messages.
Results of changes provided in this pull request are presented below:
When the self-signed certificate generating fails (in this example the timeout was still implemented) the error is printed in red color right under the general failure message:
After removing the timeout completely, the self-signed certificate is created without any issue:
Microsoft Reviewers: Open in CodeFlow