Repository navigation
Invoke-Restmethode won't timeout #12249
Description
Steps to reproduce
Hello,
i got this code here for demonstration:
Invoke-RestMethod -uri "https://XXXXXX.blob.core.windows.net/XXXXXXXX/XXXXXX/SOURCES/PE10.WIM?st=2020-02-20T13%3A08%3A43Z&se=2021-02-21T13%3A08%3A00Z&sp=racwdl&sv=2018-03-28&sr=b&sig=SIGNATURE" -OutFile "C:\Users\testUser\Downloads\DL\PRG\test.wim"
when i use that code i can get the download.
Firstly to maybe make clear what the parameter TimeoutSec should do. i thought it the connection stops i get an timeout after 10 seconds? or do i get an timeout after 10 seconds when i cant get an answer ?
To the problem: when i use that code above the download starts with Powershell 7. When i unplug my Lan cable when the download is already progressing, i can reattach the cable in maybe 10 seconds and the download resumes. when i leave it unpluged this happens:
in Powershell 5: i get an timeout after 5 minutes. and the script goes on (thats what i want)
in Powershell 7: nothing happens, waiting 5, 10 minutes, even hours (tested till 1 day) and its stuck. also the script is stuck and wont progress anymore
Expected behavior
- after the connection is lost during an download (or cable is unpluged) a timeout should happen after 5 minutes, like it happens in Powershell 5
# Actual behavior
- Powershell 7 is stuck during the download when cable is unpluged. Script hangs forever
Environment data
Activity
iSazonov commentedon Apr 4, 2020
KrX3D Please share PSVertionTable as issue template requested.
Also please test with latest build PowerShell 7.1 Preview1.
KrX3D commentedon Apr 6, 2020
Ah sorry here it is for PS7 which does not work:
And here for PS5 which does work:
iSazonov commentedon Apr 6, 2020
Please check with 7.1 Preview1
KrX3D commentedon Apr 6, 2020
i tried it with the preview version. Still the same problem:
KrX3D commentedon Jun 3, 2020
Hello,
anything new about this problem?
iSazonov commentedon Jun 3, 2020
I think it is easy to implement.
The method
PowerShell/src/Microsoft.PowerShell.Commands.Utility/commands/utility/WebCmdlet/StreamHelper.cs
Line 278 in c22ccbe
accept a cancelation token so CancellationTokenSource could be initialized with a default timeout like 300 seconds.
vexx32 commentedon Jun 3, 2020
We should also ensure that
-TimeoutSecis respected there as well, but a sensible default would be wise.iSazonov commentedon Jun 3, 2020
TimeoutSec is used in HttpClient as connection timeout.
KrX3D commentedon Jun 4, 2020
ok, but still the Invoke-Rest Comand should timeout after 5 minutes in PS7 like it does in PS5 and not hang for eternity
KrX3D commentedon Jul 22, 2020
i updated my first post and removed the timeout parameter.
but like i said, with no connection the invoke-restmethode function is stuck in powershell 7 (tested with 7.0.3) and in PS5 it just "timeouts" after 5 minutes and my script can go on
10 remaining items
iSazonov commentedon Mar 13, 2023
I said about HttpClient implementation.
PowerShell docs say about the parameter:
I take it that if during any operation any request hangs for this time (network loss), then the cmdlet should terminate.
The .Net does not seem to support this (at least in HttpClient).
So we could only implement Ctrl-C, the easiest way is to put everything into a new task with separate CTS.
stevenebutler commentedon Mar 14, 2023
Ilya (@iSazonov) it should be possible to do this without using a separate task provided the cancellation token we already have is passed through to low levels. I believe my WIP PR already demonstrates this for the CTRL-C issue and will need some additional work to support timing out when no traffic is received for a period of time (TimeoutSec).
The reason HttpClient timeout is not in play anymore is because once data is being read from the stream directly, the HttpClient API is no longer being used. The streams APIs used to do this have cancellation support if the Async APIs are used, so it is possible to implement timeouts using the cancellation token while reading from the streams.
Because of this I don't think the CmdLet needs to, or even should use a separate task to support cancellation and timeouts.
If the task is cancelled it will return to the user, but unless the cancellation token also cancels the underlying operations, they will continue even though the cmdlet appears to be finished. If the cancellation token is passed down to those underlying operations instead, when the cancellation token is cancelled it will abort the underlying operations.
The trick with this issue is that rather than using a straight timeout, the cancellation token timeout needs to be reset whenever data is received on the connection. If the cancellation token source is available to the underlying layers that are reading from the socket, it is possible to reset the timeout every time data is read by calling cts.CancelAfter(timeoutVal).
The second problem with using a task in the cmdlet is it will not be able to write progress or output to the stream from that task directly since it will likely be running in a separate thread and PowerShell does not support using WriteXXX APIs outside the main thread if I recall correctly.
I haven't tried to implement this as I'd like to work on PR #19315 which is simpler and fixes the CTRL-C support first before implementing a fix for this issue in the interests of keeping the PR small and easier to review.
iSazonov commentedon Mar 15, 2023
The problem is that when we pass a CTS into the .Net API it does not guarantee that this CTS will work as we expect, because there can be many layers within that API.
In particular I discussed this problem for the HttpClient in the .Net Runtime repository. But the same problem could be with any other API.
So it doesn't make sense to convert the code to async and distribute CTS. Eventually it will fix all the problems and will unpredictably depend on the implementation in .Net.
What we can do is explicitly solve the problem at hand. In particular, add support for Ctrl-C.
stevenebutler commentedon Mar 15, 2023
The .NET APIs that take cancellation tokens do work with the cancellation token. The issue you're describing is that HttpClient and handler, take timeouts properties but don't cancel the request when they are no longer effectively controlling them because they have handed off the response stream to be read independently. The HttpClient is not providing cancellation token to the underlying stream to cancel them because it is not reading them. The streams APIs take the cancellation token in the supported API calls and don't save a reference to the token so if they are read without passing a token they cannot be cancelled. When reading the streams if no cancellation token is passed to them, they have no way of being cancelled.
If a separate task is used, how would the supervisor then monitor the stream for activity so it can reset the cancellation timeout whenever traffic is received? The requirement is for a timeout on no stream activity over a period of time (the TimeoutSec), not a straight-forward timeout from the start of the response.
.NET core onwards is heavily biased towards cancellable asynchronous APIs so we should take advantage of them where it makes sense.
Please have a look at PR#19330 which uses the cancellable APIs to make CTRL-C work. With some additional logic to reset the cancellation token timeout whenever data is read from the stream it should be possible to do similar for this issue.
iSazonov commentedon Mar 16, 2023
They work. But not always as you expected. See dotnet/runtime#74568
In other words, your PR does not solve the Ctrl-C problem completely.
And if so, it is better not to complicate the code and apply a more universal solution.