Return Request context cancelled error#93
Conversation
|
Thank you for submitting this PR!
Getting other community members to do a review would be great help too on complex PRs (you can ask in the chats/forums). If you are unsure about something, just leave us a comment.
We currently aim to provide initial feedback/triaging within two business days. Please keep an eye on any labelling actions, as these will indicate priorities and status of your contribution. |
|
@hannahhoward The test that is failing on the CI succeeds locally for me a 100 times: Is this a known flaky test ? |
hannahhoward
left a comment
There was a problem hiding this comment.
LGTM - don't worry about that test, yes that one particular test is flaky and I've been unable to fix it from being that way. apologies.
* Emit events with received cids (#71) * persist received cids on channel state. * Send, Receive and Validate Restart requests (#75) * Send, Receive and Validate Requests * Initiating and Responding Tests and bug fixes (#76) * Testing for resuming data transfer work * Cleanup Push Restarts PR (#79) * cleanup of restart PR * link the peers * Tests for pull restarts (#84) * tests for pull restarts * Merge Tests cleanup work (#92) * cleanup of restart PR * cleanup timedout channels (#93) * backward compatibility of restart (#96) * backward compatibility of restart * changes and tests * more tests * better error handling for restarts * feat(message): switch to cbor map encoding (#97) switch to cbor map encoding for the 1_1 message protocol * feat(channels): setup datastore migrations (#99) setup datatransfer channels so they migrate over successfully Co-authored-by: Hannah Howard <hannah@hannahhoward.net>
As discussed, if the user's request context is cancelled for whatever reason, we should write that error to the reponse error channel.
@hannahhoward Because we ignore context cancellation errors here:
go-graphsync/requestmanager/executor/executor.go
Line 148 in d88611c
we had a race wherein we'd mark a request as "completed" even though the context is cancelled because we close the response error channel here without writing anything to it. Have fixed that as well.
LMKWYT.