Conversation
cmeiklejohn
left a comment
There was a problem hiding this comment.
What is this doing precisely and why did you have to make this change?
|
Please fix commit f2a8a9d to comment out, rather than remove the code and leave a comment as to the error that's thrown when that code is enabled. This will allow us to precisely identify what we removed and why. |
| # we should be able to avoid this -- it's because we're returning an invalid | ||
| # object through the opentelemetry instrumentation. | ||
| # | ||
| result = Response() |
There was a problem hiding this comment.
I’m still confused here. You shouldn’t be picking the type you want to return here. This should be the type of response that is returned when Redis naturally returns an error. Therefore, whether or not Redis actually experiences an error or we use fault injection to create one it returns the same type.
There was a problem hiding this comment.
For example, when you are issuing HTTP requests with requests, three different things can happen:
- First, the call might succeed, and it returns to the caller an instance of
Responsewith the fields set appropriately. - Second, the call might fail because of timeout or connection error. In this case, it doesn’t return anything, but instead throws an exception in the
requests.exceptionsnamespace.` - Third, the call might return a response indicating error. For example, the remote service might return HTTP 404 NotFound or HTTP 500 InternalServerError. In this case, a
Responseobject is returned with the appropriate status code and body set.
Therefore, the first thing is to figure out what the type of object being returned back to the caller, using the Redis library is. That’s the type you’ll want to use. When injecting faults that are the non-throwable kind, you’ll also want to ensure you instantiate that type and make it look — when injecting a fault that we create — indistinguishable from an actual fault.
No description provided.