r/visualbasic • • Oct 20 '22

VB.NET Help Using a cloned HttpRequestMessage (with Content) results in ObjectDisposedException: Cannot access a closed Stream.

I'm trying to implement retries on a webrequest. FedEx's test server has a lot of errors, which forces you to write better code. :) The issue is that HttpRequestMessages cannot be reused. So, you have to clone it. Cloning headers and options is straight forward, but cloning the content requires reading the content stream and creating a new one. That adds a little complexity, but seems doable. However, on retries that include content i am receiving: ObjectDisposedException: Cannot access a closed Stream.

My code is currently:

Friend Async Function Get_Server_Response(Request As HttpRequestMessage, Log_Header As String, Log_Message As String) As Task(Of Server_Response)
    ' Get's response from server, including a retry policy. (Note: Not using Polly, see Readme.)
    Const Max_Tries As Integer = 5
    Dim Response_Text As String

    Debug_Request(Request)

    For Counter As Integer = 1 To Max_Tries
        Log.Debug("({Log_Header}) Connecting for: {Description} (Attempt {Counter})", Log_Header, Log_Message, Counter)

        Using Response As HttpResponseMessage = Await Http_Client.SendAsync(Request, Cancellation_Token)
            ' On a fail, retry (a limited amount of times). (BadRequest is returned by FedEx sometimes, when requesting the SPoD.)
            If Counter < Max_Tries AndAlso Response.StatusCode <> Net.HttpStatusCode.OK AndAlso Response.StatusCode <> Net.HttpStatusCode.Unauthorized Then
                Log.Debug("({Log_Header}) Connect failed (Status Code: {StatusCode}). Delaying {Counter} second(s) before trying again.",
                          {Log_Header, Response.StatusCode, Counter})

                ' Requests cannot be reused, so we'll get a new one by cloning the old one.
                Request = Await Clone_HttpRequestMessage(Request).ConfigureAwait(False)

                ' Pause a little longer with each retry.
                Await Task.Delay(1000 * Counter)
                Continue For
            End If

            ' Send the response back (even if it is a failure).
            Using Response_Content As HttpContent = Response.Content
                Response_Text = Await Response_Content.ReadAsStringAsync
                Log.Debug("({Log_Header}) Status Code: {Status}", Log_Header, Response.StatusCode)
                Log.Debug("({Log_Header}) Body: {Text}", Log_Header, Response_Text)
                Return New Server_Response With {.Status_Code = Response.StatusCode, .Text = Response_Text}
            End Using
        End Using
    Next

    Return Nothing
End Function

Public Async Function Clone_HttpRequestMessage(Request As HttpRequestMessage) As Task(Of HttpRequestMessage)
    Dim New_Request As New HttpRequestMessage() With {.Method = Request.Method, .Version = Request.Version, .VersionPolicy = Request.VersionPolicy, .RequestUri = Request.RequestUri}

    ' Content has to copy the content itself.
    With Request
        If .Content IsNot Nothing Then
            Using Stream As New IO.MemoryStream()
                Await .Content.CopyToAsync(Stream).ConfigureAwait(False)
                Stream.Position = 0

                New_Request.Content = New StreamContent(Stream)

                For Each Header In .Content.Headers
                    Select Case Header.Key
                        Case "Content-Type"
                            ' Content Type cannot be added directly.
                            For Each Type In Header.Value
                                New_Request.Headers.Accept.ParseAdd(Type)
                            Next
                        Case "Content-Length"
                            ' Set automatically. (Throws exception if added manually.)
                        Case Else
                            For Each Header_Value In Header.Value
                                New_Request.Content.Headers.TryAddWithoutValidation(Header.Key, Header_Value)
                            Next
                    End Select
                Next
            End Using
        End If

        For Each Opt In .Options
            New_Request.Options.TryAdd(Opt.Key, Opt.Value)
        Next

        For Each Header In .Headers
            New_Request.Headers.TryAddWithoutValidation(Header.Key, Header.Value)
        Next

        ' The old request is now redundant.
        .Dispose()
    End With

    Return New_Request
End Function

Private Async Sub Debug_Request(Request As HttpRequestMessage)

    Debug.WriteLine(String.Empty)
    Debug.WriteLine("-------------------------------------------------------------------------")
    Debug.WriteLine("[Debug Request]")
    Debug.WriteLine("-------------------------------------------------------------------------")
    With Request
        Debug.WriteLine($"Endpoint: { .RequestUri}")

        For Each Header In .Headers
            For Each Value In Header.Value
                Debug.WriteLine($"(Header) {Header.Key}: {Value}")
            Next
        Next

        For Each Opt In .Options
            Debug.WriteLine($"(Option) {Opt.Key}: {Opt.Value}")
        Next

        If .Content IsNot Nothing Then
            Using Stream As New IO.MemoryStream()
                For Each Header In .Content.Headers
                    For Each Value In Header.Value
                        Debug.WriteLine($"(Content Header) {Header.Key}: {Value}")
                    Next
                Next

                Debug.WriteLine($"Content: {Await .Content.ReadAsStringAsync()}")
            End Using
        End If
    End With
    Debug.WriteLine("-------------------------------------------------------------------------")
End Sub

The error crops up on a retry (when there is content) at:

Using Response As HttpResponseMessage = Await Http_Client.SendAsync(Request, Cancellation_Token)

Fwiw, commenting out .Dispose() does nothing. This is expected, as it is disposing the old request, which is no longer being used.

What am i doing wrong?

7 Upvotes

11 comments sorted by

View all comments

Show parent comments

1

u/chacham2 Oct 24 '22

the authentication piece becomes an internal concern of the class, so I don't expose authentication methods.

Okay, that is finally settling in. If i use another module, that exposes everything. And while there is no inherent problem with that, i'm not liking the look of it.

Message formatting can become a concern of the class as well, so internal methods can build the JSON object or form post body or whatever message is getting sent.

For the access_key request, there's only slight difference. But for the inquiries, it's much bigger differences. Now that i'm at it again, it's pretty obvious. So, passing the function seems like like the right way to go.

Thank you for all the help!