Skip to content

Implement std::error::Error for SocketParseError #3816

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

Merged

Conversation

GaelicGabe
Copy link

SocketParseError does not implement std Error which means it can't be used in standard ways like combining ? and anyhow. This is problematic since it's a public interface and being able to use anyhow on public errors is nice.

This commit implements std::error::Error for SocketParseError

@ldk-reviews-bot
Copy link

ldk-reviews-bot commented May 30, 2025

👋 I see @TheBlueMatt was un-assigned.
If you'd like another reviewer assignemnt, please click here.

Copy link

codecov bot commented May 30, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 89.88%. Comparing base (8aae34e) to head (0984ea4).
Report is 32 commits behind head on main.

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #3816      +/-   ##
==========================================
+ Coverage   89.77%   89.88%   +0.10%     
==========================================
  Files         159      160       +1     
  Lines      128910   129307     +397     
  Branches   128910   129307     +397     
==========================================
+ Hits       115729   116224     +495     
+ Misses      10484    10384     -100     
- Partials     2697     2699       +2     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@ldk-reviews-bot
Copy link

🔔 1st Reminder

Hey @TheBlueMatt! This PR has been waiting for your review.
Please take a look when you have a chance. If you're unable to review, please let us know so we can find another reviewer.

Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Sure, why not.

Copy link
Contributor

@tnull tnull left a comment

Choose a reason for hiding this comment

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

Ah, please go ahead and squash the fixup so that all CI checks pass.

@TheBlueMatt TheBlueMatt removed their request for review June 2, 2025 13:03
SocketParseError does not implement std Error which means it can't be
used in standard ways like combining ? and anyhow.

This commit implements std::error::Error for SocketParseError

Hide the std::error::Error for SocketAddressParseError implementation
behind the "std" feature since it relies on the standard library.
@GaelicGabe GaelicGabe force-pushed the impl-error-for-socket-parse-error branch from e226094 to 0984ea4 Compare June 4, 2025 21:37
@GaelicGabe
Copy link
Author

Ah, please go ahead and squash the fixup so that all CI checks pass.

Done

@tnull tnull merged commit 97f0e4b into lightningdevkit:main Jun 5, 2025
27 checks passed
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.

4 participants