improvement: add timeout to requests #5452
Conversation
|
We want to add |
kasiaMarek
force-pushed
the
request-timeout
branch
2 times, most recently
from
27537cf to
5d86988
Compare
kasiaMarek
force-pushed
the
request-timeout
branch
from
5d86988 to
4314047
Compare
| import scala.meta.internal.metals.CancelableFuture | ||
| import scala.meta.internal.metals.MetalsEnrichments._ | ||
|
|
||
| object FutureWithTimeout { |
There was a problem hiding this comment.
I wonder if we could go another direction. Instead of relying on Await, we could save the request time and if another request comes in the queue and the timeout is calculated to be too much (using FlexTimeout) we could ask the user if they want to cancel it:
$name request is taking longer than expected, do you want to cancel and rerun it?
Cancel | Wait | Wait always
If they wait we could triple the flex time and ask again at some point. Wait always would imply that they don't want to show those pop ups, which could be safe if the feature is not 100 % at the start.
kasiaMarek
force-pushed
the
request-timeout
branch
from
4314047 to
8d006d4
Compare
kasiaMarek
force-pushed
the
request-timeout
branch
from
8d006d4 to
7c0251d
Compare
| @@ -211,6 +226,7 @@ class BuildServerConnection private ( | |||
| register( | |||
| server => server.buildTargetScalaMainClasses(params), | |||
| onFail, | |||
| defaultTimeout("main classes"), | |||
There was a problem hiding this comment.
One thing I realised is that, if a user is hit with that message, they wouldn't know what is going on. Most likely it's enough to show notification on compilation and otherwise cancel main/test automatically with a warning. What do you think?
There was a problem hiding this comment.
Agreed. Let's not ask in such case.
| ](compile, "cascadeBatch", shouldLogQueue = true, Some(Map.empty)) | ||
| ]( | ||
| compile(timeout = None), | ||
| "cascadeBatch", |
There was a problem hiding this comment.
Should we cancel this if we cancel normal compilation also? Since most likely this will also not finish. Cascade compile is normal compile, but on more targets.
There was a problem hiding this comment.
I set the minimum timeout to 15 min for that.
…to cascade compile
In some erroneous situations build server fails to respond, which in metals for e.g. compilation blocks the queue. This PR adds timeout to the requests, so in such situations we can recover.