Do exponential backoff for all exceptions in VMService::defaultOpenChannel. #16785
Conversation
…annel We were trying to only catch WebSocketException, but in fact SocketException can be thrown as well.
chinmaygarde
changed the title
|
|
|
Feel free to land as soon as Travis is happy. Should be a matter of updating |
| socket = await io.WebSocket.connect(uri.toString()); | ||
| } on io.WebSocketException catch(e) { | ||
| } catch (e) { | ||
| printTrace('Exception attempting to connect to observatory: $e'); |
There was a problem hiding this comment.
This means if the error was something like a CompilerError, we'll retry anyway, which is a bit weird. But I guess we're seeing a lot of different kinds of errors here?
There was a problem hiding this comment.
We can get both SocketException and WebSocketException, which for some reason don't extend one another. So I could have added two catch blocks, but I went with just a global catch. Lemme know if you think I should change it.
There was a problem hiding this comment.
As a general rule I prefer not doing global catches because they tend to swallow real errors. In this case it's not a huge deal since the real errors will still get noisily printed to the console and eventually result in a failure. I'll leave it up to you which way you want to go on this.
There was a problem hiding this comment.
What we really want here is Java multi-catch ability:
} on io.SocketExceptiojn, io.WebSocketException catch (e) {
...
}But that doesn't exist. Given the limited scope of what's in this try/catch, I'm fine with leaving it as-is, but I don't feel strongly either way.
…annel. (flutter#16785) We were trying to only catch WebSocketException, but in fact SocketException can be thrown as well.
5 participants
We were trying to only catch WebSocketException, but in fact
SocketException can be thrown as well.