tls: trace errors can show up as SSL errors #27841
Conversation
Trott
left a comment
There was a problem hiding this comment.
Remove the entry for the flaky test from test/parallel/parallel.status?
|
Looks like it needs a rebase? |
3b4cb96 to
edec9af
Compare
Calls to TLS_trace might leave errors on the SSL error stack, which then get reported as SSL errors instead of being ignored. Wrap TLS_trace to keep the error stack unchanged. Fixes: nodejs#27636
edec9af to
baff45b
Compare
|
Thank you for addressing my CR. |
|
The test is still failing in CI. Not sure if that's expected (as perhaps it has multiple issues) or if that's a sign that this isn't quite a complete fix? |
|
@Trott latest ci was all green, but I'll kick off another. |
|
test-cpu-prof failed on FreeBSD, but that doesn't seem related. Resuming. |
|
@Trott I am not seeing signs of instability. Can I land this? |
OMG yes, please! Sorry my erroneous observation caused a delay here. |
|
(Oh, wait, it wasn't erroneous. At the time, the most recent CI was https://ci.nodejs.org/job/node-test-pull-request/23424/ which shows the test still failing. But whatever, if it's all green now, let's land. Maybe this was needed in combination with something else that landed recently. |
|
Landed in b1bd9e3 |
Calls to TLS_trace might leave errors on the SSL error stack, which then get reported as SSL errors instead of being ignored. Wrap TLS_trace to keep the error stack unchanged. Fixes: #27636 PR-URL: #27841 Reviewed-By: Ben Noordhuis <info@bnoordhuis.nl> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com> Reviewed-By: Anna Henningsen <anna@addaleax.net> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Refael Ackermann <refack@gmail.com>
Calls to TLS_trace might leave errors on the SSL error stack, which then
get reported as SSL errors instead of being ignored. Wrap TLS_trace to
keep the error stack unchanged.
Fixes: #27636
Checklist
make -j4 test(UNIX), orvcbuild test(Windows) passes