fix: keep processing a block's events after encountering a dispatch error #310
Conversation
sander2
force-pushed
the
fix/subscription-event-after-error
branch
from
4f690b7
to
56c82d0
Compare
|
I have the impression we have made some similar fix locally. In the original code, it recognizes an error produced by any extrinsic including the one we don't care and fails too early. |
There was a problem hiding this comment.
LGTM.
This seems to be another hangover from when this block of code was designed only to return the events triggered by a specific extrinsic, in which case it wouldn't have been a bug since it would have skipped over any events/errors which were not for that extrinsic.
We should consider adding some unit tests for this block of code since it is so critical and has had a few bugs in it. One way to do it would be to add a Mock variant to EventStorageSubscription.
|
We have an integration test for this in our client code, but I wasn't able to replicate the test in the subxt repo - I didn't manage to create a single block with 1 failing and 1 succeeding tx. Fwiw, we've been using this patch locally for a long time, so it's been somewhat battle-tested already. I'll look into adding a unit test though |
|
@ascjones I added some unit tests. I had to refactor a little bit, because not only did I need to mock I also added a modified version of #283 based on your comment, so that I could integrate the test for it right away. If you prefer separate prs, I'm happy to remove the last commit here. |
sander2
force-pushed
the
fix/subscription-event-after-error
branch
from
72078c6
to
902d94c
Compare
Yes separate PR please |
sander2
force-pushed
the
fix/subscription-event-after-error
branch
from
902d94c
to
acb1a3f
Compare
Ok, updated. I'll update the other pr after this one is merged then. |
…rror (paritytech#310) * fix: keep processing a block's events after encountering a dispatch error * test: unit test for subscription
When processing a block, the code immediately returned any encountered errors. Any events produced in the block after the error would be silently dropped. This pr changes the behavior: an error is now handled in the same way as events. That is, they are added to the
VecDequeand are returned only when they get popped from there. This ensures that all events from the block are correctly processed