You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
Check NetworkRequest#mDestroyed before each network request? #563
I recently ran into an issue while trying to make a network request on an NextcloudAPI instance, on which I called close() before.
Of course, it failed, but at an odd place, that I didn't expected (I think context was null at some place).
This made me wondering why we do check for NetworkRequest#mDestroyed when connecting, but not before each request. Wouldn't make it sense to add this check also at the beginning of AidlNetworkRequest#performNetworkRequestV2 and throw a more meaningful IllegalStateException or something alike?
Yes, that makes perfect sense to me. Would for sure improve error handling a lot (and make it easier for devs to find the error in case something goes wrong).
My current understanding is that NetworkRequest#connect() already checks mDestroyed, but a request through AidlNetworkRequest#performNetworkRequestV2() can still be attempted after close(). Since close() marks the API as destroyed and clears its state, this can result in an unrelated exception instead of clearly indicating that the API can no longer be used.
I’m planning to add an mDestroyed check at the beginning of performNetworkRequestV2() and throw a meaningful IllegalStateException.
If this approach sounds good, I’d be happy to work on it.
Activity
David-Development commentedon May 29, 2023
Yes, that makes perfect sense to me. Would for sure improve error handling a lot (and make it easier for devs to find the error in case something goes wrong).
Abhijeet-035 commentedon Sep 6, 2026
Hi @David-Development, @stefan-niedermann! I’d like to work on this issue.
My current understanding is that
NetworkRequest#connect()already checksmDestroyed, but a request throughAidlNetworkRequest#performNetworkRequestV2()can still be attempted afterclose(). Sinceclose()marks the API as destroyed and clears its state, this can result in an unrelated exception instead of clearly indicating that the API can no longer be used.I’m planning to add an
mDestroyedcheck at the beginning ofperformNetworkRequestV2()and throw a meaningfulIllegalStateException.If this approach sounds good, I’d be happy to work on it.