-
Notifications
You must be signed in to change notification settings - Fork 220
Description
In #172, when making the logging of network responses be a bit lazy, I noted:
In theory, this could make the logs less useful, by adding a delay between
when the network response was actually received, and when it is logged. Or
the response may never be logged, if the content is never accessed. In
practice, this is unlikely to happen, because nearly all SDK methods
immediately read the content.
It turns out that this assessment wasn't quite correct. Most SDK methods do immediately read the content.. on success. On failure, the session doesn't always read the content. For example:
- For most auth failures, we don't read the error response before attempting a refresh.
- For retries on 429 and 5xx responses, we don't read the error response before attempting a retry.
If the SDK knows that it is throwing away a request, it should read the content first. That way, if the logging network is being used, it can be logged.
Alternatively, we can call del response, and then have a __del__ on LoggingNetworkResponse which does the content reading. That way, the BoxSession code doesn't need to be too closely tied to the logging network (though still a bit tied, because that's the only reason for having del response). This is somewhat related to #189. If retries were iterative instead of recursive, then the old response would have its refcount go to 0 automatically, which would eliminate the need to manually call del response.