Updated handling of gRPC Context Cancelled#159
Updated handling of gRPC Context Cancelled#159nrjpoddar wants to merge 1 commit intoweaveworks:masterfrom
Conversation
Signed-off-by: Neeraj Poddar <neeraj@aspenmesh.io>
|
This PR #151 was supposed to fix it but I noticed that the logging was still happening on Cancelled contexts as the error checking was incorrect. |
|
@bboreham do you mind taking a look? |
|
The code looks fine, but given the previous one also looked fine, can we get any more assurance this time? |
|
Haha, totally agree and a valid point. How did you find out what to do, both times? Can we point to a reference? -> First time I didn't understand correctly what error type was being returned here for context cancellations. After we upgraded ingester in our environment which had my fixes I was still seeing this error:
From this I derived that the error was a gRPC error type. Unit test for this might be a bit tricky but I think I can write it with sufficient dependency injection. |
|
Having extended the tests to cover cancellation, I believe this PR was incorrect - we need to cover both |
Signed-off-by: Alex Yu <yu.alex96@gmail.com>
Signed-off-by: Neeraj Poddar neeraj@aspenmesh.io