KOL-5 | feat: Enhance error handling with detailed KolmeError variants #403
Conversation
Deploying kolme with
|
| Latest commit: |
bf28299
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://88566749.kolme.pages.dev |
| Branch Preview URL: | https://kol-5.kolme.pages.dev |
There was a problem hiding this comment.
Left some comments for a change in general direction. When this is ready for another review, let's get @MichaelNelo on this too so I'm not a bottleneck.
anakinzhed
force-pushed
the
KOL-5
branch
2 times, most recently
from
2efdafe to
a75f49d
Compare
| processor.add_secret(new_processor.clone()); | ||
| testtasks.try_spawn_persistent(processor.run()); | ||
|
|
||
| // The genesis event hasn't completed, which causes this test to fail. |
There was a problem hiding this comment.
We need to figure this one out before merging this PR, this seems like a new regression since the tests are currently passing without the addition.
There was a problem hiding this comment.
Still working on this. I haven't found the reason.
| Other(String), | ||
| } | ||
|
|
||
| // CREATE A GENERIC, MACRO OR SOMETHING WITH SIMILAR BEHAVIOR |
There was a problem hiding this comment.
Check the docs for thiserror, IIRC the #[from] pragma can be used.
There was a problem hiding this comment.
As far as I can see, I can’t use something like:
#[error("CoreState error: {0}")]
CoreState(#[from] CoreStateError),
because KolmeError derives Clone, Serialize, and Deserialize, while most of the underlying errors do not implement these traits, which causes a compilation error. If I remove the Clone, Serialize, and Deserialize derives from KolmeError, a lot of things start breaking at different levels. Let me know if you’d like me to remove those derives.
| ) -> Result<()> { | ||
| anyhow::ensure!(self | ||
| ) -> std::result::Result<(), KolmeError> { | ||
| kolme_ensure!(self |
There was a problem hiding this comment.
It looks like this kolme_ensure is essentially being used to bypass the need for creating specific error variants. Instead, I'd want to see variants for the specific failure modes. To avoid creating one massive enum with hundreds of variants, a common approach would be defining a helper type, like KolmeExecuteError, in this module, and then providing an Execute(KolmeExecuteError) variant within KolmeError.
There was a problem hiding this comment.
kolme_ensure! is simply replacing anyhow::ensure! at all different levels. For instance:
kolme_ensure!(info.validator_set.processor == processor);
kolme_ensure!(listeners == info.validator_set.listeners);
kolme_ensure!(needed_listeners == info.validator_set.needed_listeners);
kolme_ensure!(approvers == info.validator_set.approvers);
kolme_ensure!(needed_approvers == info.validator_set.needed_approvers);
I don't understand your request.
2 participants
No description provided.