Feature anyhow removal by arkanoider · Pull Request #459 · MostroP2P/mostro

arkanoider · 2025-03-02T10:58:05Z

removed anyhow dependency.

Summary by CodeRabbit

Chores
- Removed the anyhow dependency to streamline the project.
Refactor
- Unified error handling across core functionalities—such as payment processing, order management, and configuration setup—to deliver more precise and consistent error messages.
- Updated various function signatures to utilize MostroError for enhanced error specificity.
- Improved error handling in the Bitcoin price management functionality for better robustness.

coderabbitai · 2025-03-02T10:58:11Z

Walkthrough

This pull request refactors error handling across the codebase by removing the dependency on the anyhow crate and replacing its usage with a custom error type, MostroError. Function signatures in multiple modules have been updated to return Result<T, MostroError>, and import statements for anyhow::Result have been removed. In addition, the Cargo.toml file now omits the anyhow dependency.

Changes

Files	Change Summary
Cargo.toml	Removed external dependency `anyhow = "1.0.89"` and updated `mostro-core` version from `0.6.28` to `0.6.30`.
src/app.rs, src/app/add_invoice.rs, src/app/admin_add_solver.rs, src/app/rate_user.rs, src/app/take_buy.rs, src/app/take_sell.rs, src/lightning/mod.rs, src/main.rs, src/messages.rs	Removed `use anyhow::Result` imports; updated error handling in `src/app.rs` to pass a dereferenced error; updated module import paths.
src/bitcoin_price.rs, src/cli.rs, src/cli/settings.rs, src/db.rs, src/scheduler.rs, src/util.rs	Changed function signatures from `anyhow::Result` to `Result<…, MostroError>`; enhanced error propagation with structured error mapping (e.g., converting errors via `map_err` into `MostroInternalErr`).

Possibly related PRs

Rabbit insipired unwrap removal #382: The changes in the main PR, which involve the removal of the anyhow dependency and modifications to error handling in various files, are related to the changes in the retrieved PR, specifically in src/app/add_invoice.rs, where error handling is also being restructured.
Feat: issue #427 avoid multiple pending orders #444: The changes in the main PR, which involve the removal of the anyhow dependency and updates to error handling across various files, are related to the retrieved PR as both involve modifications to error handling mechanisms, specifically transitioning to the MostroError type.
Rate user feature #404: The changes in the main PR, specifically the update of the mostro-core dependency version in Cargo.toml, are related to the retrieved PR, which also involves a modification to the mostro-core dependency version, indicating a direct connection at the code level.

Poem

In the code garden, I hop with delight,
Leaving anyhow behind in the night.
With MostroError shining ever so bright,
Each function now sings with purpose and might.
Hoppy coding, friends, under the moonlight! 🐰✨

📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between b5d70be and d17526c.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (1)

Cargo.toml (1 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

Cargo.toml

Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media?

❤️ Share

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai generate docstrings to generate docstrings for this PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

coderabbitai

Actionable comments posted: 0

🧹 Nitpick comments (3)

src/bitcoin_price.rs (1)
43-51: Enhanced return type for get_price method.

The method has been improved to return Result<f64, MostroError> instead of Option<f64>, which provides better error information when price retrieval fails. The error handling is properly implemented using map_err and ok_or.

Consider adding a more specific error message when a currency is not found, such as including the currency code in the error:
- .ok_or(MostroInternalErr(ServiceError::NoAPIResponse))
+ .ok_or(MostroInternalErr(ServiceError::NoAPIResponse(format!("Price for currency '{}' not found", currency))))
This would require updating the ServiceError enum to accept a String for the NoAPIResponse variant.
src/db.rs (2)
590-591: Remove debug print statements.

These println statements appear to be leftover debugging code that should be removed or replaced with proper logging.
-    println!("solver_pubkey: {}", solver_pubkey);
-    println!("order_id: {}", order_id);
+    // Use tracing module for logging if needed
+    tracing::debug!("Checking if solver {} is assigned to order {}", solver_pubkey, order_id);
22-624: Consider extracting the common error mapping pattern into a helper function.

Throughout the file, there's a repetitive pattern of mapping SQLx errors to MostroInternalErr. Creating a helper function would reduce duplication and make the code more maintainable.

You could create a helper function like this:
fn map_db_err<T>(result: Result<T, sqlx::Error>) -> Result<T, MostroError> {
    result.map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))
}
Then use it in your code:
-    let mut conn = pool
-        .acquire()
-        .await
-        .map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))?;
+    let mut conn = map_db_err(pool.acquire().await)?;
And for query execution:
-    .await
-    .map_err(|e| MostroInternalErr(ServiceError::DbAccessError(e.to_string())))?;
+    .await
+    .pipe(map_db_err)?;
The .pipe() method would require the tap crate or similar function composition utility, but this approach would significantly reduce code duplication.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 4176c72 and 8c21b90.

⛔ Files ignored due to path filters (1)

Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (16)

Cargo.toml (1 hunks)
src/app.rs (1 hunks)
src/app/add_invoice.rs (0 hunks)
src/app/admin_add_solver.rs (0 hunks)
src/app/rate_user.rs (0 hunks)
src/app/take_buy.rs (0 hunks)
src/app/take_sell.rs (0 hunks)
src/bitcoin_price.rs (2 hunks)
src/cli.rs (2 hunks)
src/cli/settings.rs (8 hunks)
src/db.rs (34 hunks)
src/lightning/mod.rs (0 hunks)
src/main.rs (1 hunks)
src/messages.rs (1 hunks)
src/scheduler.rs (2 hunks)
src/util.rs (1 hunks)

💤 Files with no reviewable changes (6)

src/app/take_sell.rs
src/app/take_buy.rs
src/app/add_invoice.rs
src/app/rate_user.rs
src/lightning/mod.rs
src/app/admin_add_solver.rs

✅ Files skipped from review due to trivial changes (1)

src/main.rs

🔇 Additional comments (21)

Cargo.toml (1)

53-54: LGTM! The patch configuration is a good approach for local development.

The PR successfully removes the anyhow dependency and adds a local path reference to mostro-core in the patch section. This setup is beneficial for development as it allows you to work with a local version of the core library while implementing the error handling changes.

src/app.rs (1)

356-356: Fixed error handling to dereference the MostroError correctly.

The code properly dereferences the error with *err when passing it to the manage_errors function. This change is necessary after switching from anyhow::Result to the custom MostroError type.

src/cli.rs (3)

5-5: LGTM! Appropriate import for the new error handling approach.

This import properly replaces the previous anyhow::Result dependency with the project-specific error type.

32-32: Function signature updated to use the custom error type.

The return type now explicitly specifies MostroError as the error type, which enhances clarity and type safety.

36-36: Function return values properly wrapped with Ok.

The return values are now properly wrapped with Ok() to match the new function signature. This ensures type consistency throughout the error handling chain.

Also applies to: 38-38

src/messages.rs (2)

1-1: LGTM! Appropriate import for the new error handling approach.

This import properly replaces the previous anyhow::Result dependency with the project-specific error type.

7-7: Function signature updated to use the custom error type.

The return type now explicitly specifies MostroError as the error type, which enhances clarity and type safety.

src/scheduler.rs (4)

12-13: Appropriate import of custom error types.

Good addition of imports for the custom MostroError and ServiceError types that are part of the PR's main goal to replace anyhow.

235-235: Updated function signature correctly.

The function signature has been properly updated to return Result<(), MostroError> instead of anyhow::Result<()>, which aligns with the PR objective of removing anyhow dependency.

240-244: Improved error handling with specific error types.

Error handling has been properly refactored to use the custom error type hierarchy. This provides more specific error information compared to the previous anyhow implementation.

248-252: Consistent error handling pattern.

The error handling for key retrieval follows the same pattern as the database connection error handling, maintaining consistency throughout the codebase.

src/util.rs (1)

542-542: Updated return type for invoice_subscribe function.

The function signature has been properly changed from anyhow::Result<()> to Result<(), MostroError> to align with the PR objective of removing anyhow dependency.

src/bitcoin_price.rs (3)

1-2: Proper imports for custom error types.

Good addition of the necessary imports for MostroError and ServiceError to implement the anyhow removal strategy.

23-31: Enhanced error handling in update_prices method.

The method's return type has been correctly changed to Result<(), MostroError> and HTTP request error handling has been improved with specific error types, making debugging and error handling more effective.

36-39: Consistent error handling for RwLock operations.

Error handling for RwLock write operations follows the same pattern as other error handling in the file, converting lock errors to ServiceError::IOError.

src/cli/settings.rs (5)

3-4: Appropriate imports for custom error types.

Good addition of the necessary imports to implement the anyhow removal strategy across the codebase.

50-54: Updated error type for TryFrom implementation.

The error type for the Database TryFrom implementation has been correctly updated to use MostroError, aligning with the PR objective.

70-74: Consistent error type updates across all TryFrom implementations.

All TryFrom implementations for the different configuration structs (Lightning, Nostr, Mostro) have been updated consistently to use MostroError as the error type.

Also applies to: 84-88, 107-111

173-173: Updated function signature for init_default_dir.

The function signature has been properly updated to return Result<PathBuf, MostroError> instead of using anyhow's Result type.

187-189: Enhanced error handling with specific error types.

The error handling in init_default_dir has been improved to use map_err with specific ServiceError variants (EnvVarError, IOError), providing better context for troubleshooting when errors occur.

Also applies to: 204-213

src/db.rs (1)

22-73: LGTM! Good error handling refactoring.

The removal of anyhow and introduction of the more specific MostroError type improves the API of this module by making the error types explicit in the function signatures.

arkanoider · 2025-03-02T15:26:03Z

Ok @grunch removed from cargo.toml patch to mostro-core

Mostro core PR here

Catrya

tACK

grunch

tACK

@arkanoider

* 📝 Add docstrings to `feature-full-privacy-mode-checks` (#455) * add feature to check correctly if order is full privacy or normal * 📝 Add docstrings to `feature-full-privacy-mode-checks` Docstrings generation was requested by @arkanoider. * #454 (comment) The following files were modified: * `src/app/order.rs` * `src/util.rs` --------- Co-authored-by: arkanoider <github.913zc@simplelogin.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: arkanoider <113362043+arkanoider@users.noreply.github.com> * Fix network in order event (#452) Now in orers event 38383 the network that comes out is the correct one * Feature anyhow removal (#459) * remove of anyhow dependency start * removed anyhow dependecy * remove patch from cargo.toml * Update mostro-core version --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * Feature disputes (#463) * Feature: dispute fix for new logic * fix for coop cancel case * Add logic for info message about dispute for solver * first working dispute info message for admin after taking disputes * added fields to dispute info message for solver * removed some probably useless and refactored a bit admin-take-dispute function * clippy suggestion * other cosmetics to admin take file * some other cosmetics on admin-take-dispute file * fix on admin-take * Update admin_take_dispute.rs * fixed a wrong search of order id in admin take file@ * Fix: added correct check to add solver from admin cli * fix: wrong check on admin cancel * Fix initiator pubkey: now is the trade key of initiator not identity * quick fix for full privacy orders * new logic with mixed full privacy - regular reputation mode * Update cargo toml to compile with version 0.6.32 of mostro-core - this is in preparation for migrating to nostr-sdk 0.40 * rollback sdk to 0.38 version to merge disputes - we will come back to 0.40 when we will fix signature issue * Update mostro-core dependency --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * added correction for the case of buyer adding back a new invoice after payment failure (#462) * Fix for nostr sdk 40 issue on incoming message (#465) * testing sdk 40 * fix for sdk 40 * cleaned cargo.toml * Bump mostro core version --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> * fix: sends order with an updated 'status' field as the reply to add-invoice when there is a preimage (#464) * Feature-nip69-order-status (#467) * feature: align to nip69 order status in nostr event * Improved in progress nip69 logic * refined event states of the order with nip69 request * add another check to avoid multiple events with in-progress state * Privacy range order fix (#468) * add optional field for users in full privacy inside solver message * To be tested - privacy range child order fix * fix cargo.toml * bumped mostro-core version to 6.35 * New user or privacy order send same user info with zeros (#471) * new user or privacy order send same user info with zeros * fix days field and removed some println! macro with nicer tracing messages * bumped mostro-core version to 6.36 and added requested fix to compile * Add previous order state in database for solver message (#472) * add previous state in database for solver message * bumped mostro-core version to 6.36 * Taker info message to maker. I think that could be modifies in a more generic UserInfo. (#473) * added logic to send infos to maker when an order is taken * Add takers info message for maker * fix cargo.toml * set peer pubkey to empty string when notifying taker reputation to maker * moved user info message to the right point of the order flow * fix cargo.toml * changed message struct from UserDisputeInfo to UserInfo * fix: get master keys for user infos * Bumped mostro-core to version 6.38 * fix for buy order flow with message to maker (#479) * Rabbit fixes --------- Co-authored-by: Francisco Calderón <fjcalderon@gmail.com> Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com> Co-authored-by: Catrya <140891948+Catrya@users.noreply.github.com> Co-authored-by: Bilthon <bilthon@gmail.com>

arkanoider added 2 commits February 26, 2025 22:01

remove of anyhow dependency start

e8f7226

removed anyhow dependecy

8c21b90

coderabbitai Bot reviewed Mar 2, 2025

View reviewed changes

grunch reviewed Mar 2, 2025

View reviewed changes

Comment thread Cargo.toml Outdated

grunch requested a review from Catrya March 2, 2025 12:48

remove patch from cargo.toml

b5d70be

arkanoider requested a review from grunch March 2, 2025 15:31

Catrya approved these changes Mar 3, 2025

View reviewed changes

Update mostro-core version

d17526c

grunch approved these changes Mar 3, 2025

View reviewed changes

grunch merged commit 78e1dcc into develop Mar 3, 2025

grunch deleted the feature-anyhow-removal branch March 3, 2025 19:59

coderabbitai Bot mentioned this pull request Apr 30, 2025

Develop branch merge to main #480

Merged

coderabbitai Bot mentioned this pull request Aug 10, 2025

Start Os wrapper helper #503

Merged

coderabbitai Bot mentioned this pull request Aug 19, 2025

Restore message for client switching #512

Merged

coderabbitai Bot mentioned this pull request Jun 11, 2026

feat(price): Phase 1 — Yadio provider + PriceManager wiring #753

Merged

5 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature anyhow removal#459

Feature anyhow removal#459
grunch merged 4 commits into
developfrom
feature-anyhow-removal

arkanoider commented Mar 2, 2025 •

edited by coderabbitai Bot

Loading

Uh oh!

coderabbitai Bot commented Mar 2, 2025 •

edited

Loading

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (`.coderabbit.yaml`)

Documentation and Community

Uh oh!

coderabbitai Bot left a comment

Uh oh!

Uh oh!

arkanoider commented Mar 2, 2025

Uh oh!

Catrya left a comment

Uh oh!

grunch left a comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

arkanoider commented Mar 2, 2025 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Uh oh!

coderabbitai Bot commented Mar 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Walkthrough

Changes

Possibly related PRs

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

coderabbitai Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

arkanoider commented Mar 2, 2025

Uh oh!

Catrya left a comment

Choose a reason for hiding this comment

Uh oh!

grunch left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

arkanoider commented Mar 2, 2025 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Mar 2, 2025 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)