Skip to content

Conversation

@ivanlele
Copy link

One of the PRs in a series addressing issue #28.

In the legacy commands (address, block, tx), panic-based logic has been removed from the execution path and replaced with custom errors.

For the remaining commands, crate::cmd::simplicity::Error has been removed from the execution logic in favor of command-specific custom errors.

This change is crucial for enabling programmable interfaces, as it allows errors to be caught and handled programmatically by inspecting the underlying error chain rather than relying on catching panics or parsing of string errors.

@ivanlele
Copy link
Author

CI passed: 8384b9

@apoelstra
Copy link
Contributor

In a548ad8:

Rather than doing let inspect_fn = || -> Result can you move the logic into exec_inner functions the way that it's done in the new simplicity commands?

"020000000000000000000000000000000000000000000000000000000000000000",
],
"Execution failed: invalid pubkey: Encoding(Secp256k1(InvalidPublicKey))\n",
"Execution failed: invalid pubkey: string error\n",
Copy link
Contributor

Choose a reason for hiding this comment

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

In a548ad8:

Heh, this is unfortunate. It's ultimately a bug in rust-bitcoin's ParsePublicKeyError that its errors have a terrible Display impl. Nothing we can do here.

Copy link
Contributor

@apoelstra apoelstra left a comment

Choose a reason for hiding this comment

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

On 8384b92 successfully ran local tests

@ivanlele
Copy link
Author

In a548ad8:

Rather than doing let inspect_fn = || -> Result can you move the logic into exec_inner functions the way that it's done in the new simplicity commands?

In the Err match arm for output handling, I kept panics to trigger the existing panic hook. This preserve the same output formatting and as their old counterparts, even though the newer commands use cmd::print_output

@ivanlele ivanlele requested a review from apoelstra December 23, 2025 20:04
@apoelstra
Copy link
Contributor

Can you squash df35c9c into a548ad8 so that there isn't one commit which rewrites the diff of another?

@ivanlele ivanlele force-pushed the feature/error-handling branch from df35c9c to c3b23ec Compare December 24, 2025 14:35
@ivanlele
Copy link
Author

Can you squash df35c9c into a548ad8 so that there isn't one commit which rewrites the diff of another?

done

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.

2 participants