Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

crypto: add debug info to client emit secureConnect #28067

Closed
wants to merge 1 commit into from

Conversation

danbev
Copy link
Contributor

@danbev danbev commented Jun 5, 2019

Currently, when debugging a TLS connection there might be multiple debug
statements client emit secureConnect for the 'secureConnect' event
when using NODE_DEBUG='tls'. While it is possible to step through this
with a debugger that is not always the fastest/easiest to do if debugging
remote code.

This commit adds some additional information to the debug statements to
make it easier to distinguish where the debug statements are coming
from.

Checklist
  • make -j4 test (UNIX), or vcbuild test (Windows) passes
  • commit message follows commit guidelines

@nodejs-github-bot

This comment has been minimized.

@danbev danbev force-pushed the _tls_wrap_debug_statements branch from e815b0c to b047e57 Compare June 5, 2019 10:41
Copy link
Member

@richardlau richardlau left a comment

Choose a reason for hiding this comment

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

Commit message contains a typo: NODE_DEBUG='tsl' -> NODE_DEBUG='tls'.

Currently, when debugging a TLS connection there might be multiple debug
statements 'client emit secureConnect' for the 'secureConnect` event
when using NODE_DEBUG='tls'. While it is possible to step through this
with a debugger that is not always the fastest/easiest to do if
debugging remote code.

This commit adds some additional information to the debug statements to
make it easier to distinguish where the debug statements are coming
from.
@danbev
Copy link
Contributor Author

danbev commented Jun 5, 2019

Commit message contains a typo: NODE_DEBUG='tsl' -> NODE_DEBUG='tls'.

Thanks, I've updated the commit message now.

@danbev danbev force-pushed the _tls_wrap_debug_statements branch from b047e57 to 1a6efb6 Compare June 5, 2019 13:44
@nodejs-github-bot
Copy link
Collaborator

@nodejs-github-bot
Copy link
Collaborator

@danbev
Copy link
Contributor Author

danbev commented Jun 10, 2019

Landed in 5bad514.

@danbev danbev closed this Jun 10, 2019
@danbev danbev deleted the _tls_wrap_debug_statements branch June 10, 2019 03:37
danbev added a commit that referenced this pull request Jun 10, 2019
Currently, when debugging a TLS connection there might be multiple debug
statements 'client emit secureConnect' for the 'secureConnect` event
when using NODE_DEBUG='tls'. While it is possible to step through this
with a debugger that is not always the fastest/easiest to do if
debugging remote code.

This commit adds some additional information to the debug statements to
make it easier to distinguish where the debug statements are coming
from.

PR-URL: #28067
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
BridgeAR pushed a commit that referenced this pull request Jun 17, 2019
Currently, when debugging a TLS connection there might be multiple debug
statements 'client emit secureConnect' for the 'secureConnect` event
when using NODE_DEBUG='tls'. While it is possible to step through this
with a debugger that is not always the fastest/easiest to do if
debugging remote code.

This commit adds some additional information to the debug statements to
make it easier to distinguish where the debug statements are coming
from.

PR-URL: #28067
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Richard Lau <riclau@uk.ibm.com>
Reviewed-By: Sam Roberts <vieuxtech@gmail.com>
Reviewed-By: Yongsheng Zhang <zyszys98@gmail.com>
Reviewed-By: Trivikram Kamat <trivikr.dev@gmail.com>
Reviewed-By: Rich Trott <rtrott@gmail.com>
@BridgeAR BridgeAR mentioned this pull request Jun 17, 2019
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.

8 participants