Fix error codes #182
Fix error codes #182
Conversation
enforce much tighter consistency with graphsync spec in usage of error codes
| if err == runtraversal.ErrFirstBlockLoad { | ||
| code = graphsync.RequestFailedContentNotFound | ||
| } else if err == errCancelledByCommand { |
There was a problem hiding this comment.
It looks like in some of the error cases handled here we set the code then return nil.
But in these new error cases we don't return after setting the code. Just want to confirm that's correct?
There was a problem hiding this comment.
Yea so, basically the above two cases involve no final network message to send (see rb.ClearRequest)
While these cases do involve a final message back to the requestor. (see rb.FinishWithError). Ultimately they return nil two lines below that. This is a lot of nested conditionals and I agree it's confusing and merits a refactor, but yes, it's correct :)
There was a problem hiding this comment.
Cool just wanted to make sure 👍
| } | ||
|
|
||
| // ErrFirstBlockLoad indicates the traversal was unable to load the very first block in the traversal | ||
| const ErrFirstBlockLoad = errorString("Unable to load first block") |
There was a problem hiding this comment.
Why not just use errors.New()?
There was a problem hiding this comment.
cause then it's a VAR rather than a const. I've always been anxious about golang errs being vars. cause theoretically anyone can overwrite them, so I often employ this pattern. See https://dave.cheney.net/tag/errors
* feat: fire network error when network disconnects during request (#164) * release: 0.6.1 * Better logging for Graphsync traversal (#167) * log gs traversal * Apply suggestions from code review Co-authored-by: dirkmc <dirkmdev@gmail.com> * add debug logs * add debug logs Co-authored-by: dirkmc <dirkmdev@gmail.com> * release: v0.6.2 (#168) * Fix/log blockstore reads (#169) * log gs traversal * Apply suggestions from code review Co-authored-by: dirkmc <dirkmdev@gmail.com> * add debug logs * fixed logging * Apply suggestions from code review Co-authored-by: dirkmc <dirkmdev@gmail.com> * fixed error Co-authored-by: dirkmc <dirkmdev@gmail.com> * release: v0.6.3 (#170) * request queued hook * test request queued hook * release: v0.6.4 * Resolve 175 race condition, no change to hook timing (#178) * feat(requestmanager): add request timing (#181) * Add cancel request and wait function (#185) * fix(responsemanager): fix error codes (#182) enforce much tighter consistency with graphsync spec in usage of error codes * refactor: replace particular request not found errors with public error (#188) * release: 0.6.8 Co-authored-by: dirkmc <dirkmdev@gmail.com> Co-authored-by: Aarsh Shah <aarshkshah1992@gmail.com>
Goals
Enforce returning error codes that more closely match the graphsync spec
Implementation