okio icon indicating copy to clipboard operation
okio copied to clipboard

Throw new InterruptedIOException subtype on timeout

Open dmgd opened this issue 5 years ago • 6 comments

Introduces TimeoutException which a caller can catch if it just wants to just deal with timeout and deadline reached, but not interrupts.

dmgd avatar Feb 14 '20 08:02 dmgd

The specific case I'm dealing with is something that wants to transform timeouts into an http response, but leave interrupts alone. It could do this by catching InterruptedIOException and then working out whether it's a timeout or not + rethrowing if it's not, but this change would make the handling code much cleaner

dmgd avatar Feb 14 '20 08:02 dmgd

Unfortunately you’re competing with SocketTimeoutException which also attempts to signal whether the failure was caused by timeout.

Introducing our own timeout exception feels like a complicated feature that we’d need to do everywhere, including significant tests in Okio and OkHttp.

swankjesse avatar Feb 14 '20 16:02 swankjesse

Unfortunately you’re competing with SocketTimeoutException which also attempts to signal whether the failure was caused by timeout.

Introducing our own timeout exception feels like a complicated feature that we’d need to do everywhere, including significant tests in Okio and OkHttp.

Good point about SocketTimeoutException. We already catch and convert that, so reusing that would work perfectly for us. I didn't reuse it just because these timeouts are not really socket related, so it felt a bit wrong . . .

In terms of backwards compatibility, TimeoutException is still an InterruptedIOException and has the same message it already had, so the only thing that I can think of that could break would be comparisons using getClass() rather than instanceof

dmgd avatar Feb 14 '20 17:02 dmgd

I definitely like subtyping InterruptedIOException. The holdup is more about finding all of hte places where we should be throwing TimeoutException.

swankjesse avatar Feb 14 '20 18:02 swankjesse

So I think what I've just pushed might be everywhere that's relevant in okio (based on searches for "timeout" "deadline reached" and InterruptedIOException.) Is it the sort of thing you had in mind?

I'm taking a look at OkHttp now :)

dmgd avatar Feb 15 '20 15:02 dmgd

https://github.com/dmgd/okhttp/commit/71fa6b5e7d625fd6258a1b0c963f784aec42ac5b and https://github.com/dmgd/okhttp/commit/a9fb839811e615c81ad3b27949876c3cad71f8a4 look like the okhttp changes that would go with this

dmgd avatar Feb 17 '20 08:02 dmgd