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

fix: block filters can be also recreated #88

Closed
wants to merge 2 commits into from

Conversation

AuHau
Copy link
Member

@AuHau AuHau commented Oct 25, 2024

Adds Result to the Subscription's handlers that will return errors when any error in the underlaying subscription mechanism (for example HTTP polling) surface.

if event =? E.decode(log.data, log.topics):
handler(event)
handler(success(event))
Copy link
Contributor

Choose a reason for hiding this comment

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

Wouldn't you just want to pass in the result of E.decode? It returns ?!E already, and that way decoding errors won't get swallowed.

Comment on lines +245 to +249
if blckResult.isErr:
blockSubscriptionError = blckResult.error()
blockEvent.fire()

let blck = blckResult.value
Copy link
Contributor

Choose a reason for hiding this comment

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

Needs a return in the if block. Also, blockSubscriptionError = nil should be set somewhere to "reset" the error?

Can also do:

blockSubscriptionError = nil

without blck =? blckResult, error:
  blockSubscriptionError = error
  blockEvent.fire()

ContractError* = object of EthersError
SignerError* = object of EthersError
SubscriptionError* = object of EthersError
SubscriptionResult*[E] = Result[E, ref SubscriptionError]
Copy link
Contributor

Choose a reason for hiding this comment

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

SubscriptionResult definitely works... but sometimes it can be a more of a pain than it's worth, mostly when needing to convert between different Result types when only the error type is different (eg using some flavour of mapErr). You can still return specific error types when it needs inspection at the callsite (since they inherit from CatchableError). See the reservations module for examples on how to do this, eg https://github.com/codex-storage/nim-codex/blob/ebee85b1aae05e50839a8f7e911ea888cff209bc/codex/sales/reservations.nim#L273 -- notice the return type of updateAvailability is ?!void, but here we're returning a specific error type). This is the pattern I'd suggest: return/parameter types should ?!T, then return specific error types where needed.

@AuHau AuHau closed this Oct 31, 2024
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