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

tls: wrap ssl errors in ECONNRESET #54492

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Conversation

Uzlopak
Copy link
Contributor

@Uzlopak Uzlopak commented Aug 22, 2024

For discussion.

I think we should not break userland, by passing the OpenSSL Errors directly to userland. IMHO they need to be wrapped accordingly.

This PR is not passing all tests and is for demonstration purposes.

Especially notifying:
@mcollina
@RafaelGSS
@nodejs/undici

@nodejs-github-bot nodejs-github-bot added errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem. labels Aug 22, 2024
@Uzlopak Uzlopak mentioned this pull request Aug 22, 2024
Copy link

codecov bot commented Aug 22, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 87.33%. Comparing base (7c76fa0) to head (be1f3f9).
Report is 10 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main   #54492      +/-   ##
==========================================
+ Coverage   87.31%   87.33%   +0.01%     
==========================================
  Files         648      649       +1     
  Lines      182359   182530     +171     
  Branches    34979    35028      +49     
==========================================
+ Hits       159235   159411     +176     
+ Misses      16389    16381       -8     
- Partials     6735     6738       +3     
Files Coverage Δ
lib/_tls_wrap.js 93.37% <100.00%> (-0.02%) ⬇️
lib/internal/errors.js 94.44% <100.00%> (ø)

... and 37 files with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
errors Issues and PRs related to JavaScript errors originated in Node.js core. needs-ci PRs that need a full CI run. tls Issues and PRs related to the tls subsystem.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants