Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

PushStreamContent has unclear documentation #310

Closed
lichutin-st opened this issue Dec 8, 2021 · 3 comments
Closed

PushStreamContent has unclear documentation #310

lichutin-st opened this issue Dec 8, 2021 · 3 comments
Assignees
Milestone

Comments

@lichutin-st
Copy link

Hello!

As for me, documentation for a constructor of PushStreamContent which receives an asynchronous action looks a bit unclear.

It's said The stream is automatically closed when the return task is completed but actually it isn't. Here code waits for TaskSource.Task completition that is only caused by manual calling stream.Dispose(): link

So this content will never be completed

new PushStreamContent(async (stream, content, transport) => {
    await JsonSerializer.SerializeAsync(stream, data);
});

And we have to close the stream manually to make it work

new PushStreamContent(async (stream, content, transport) => {
    await JsonSerializer.SerializeAsync(stream, data);
    stream.Dispose();
});

You can compare the constructor with another one (receiving sync action). Docs there look more correct

@lichutin-st lichutin-st changed the title Unclear documenation for PushStreamContent PushStreamContent has unclear documentation Dec 8, 2021
@mkArtakMSFT
Copy link
Member

Thanks for contacting us.
We've reviewed the code and the task is indeed being closed. Note, that the lambda function that you're calling stream.Dispose() in is meant to simply use the stream and the disposal will happen in a different place, not within the scope of this code. It will happen when the work with the stream is complete.

@lichutin-st
Copy link
Author

Hello!
@mkArtakMSFT I'm not sure I fully got your response. If I don't dispose the stream inside the lambda, the request will never be completed.

            var content = new PushStreamContent(async (stream, content, transport) => {
                await JsonSerializer.SerializeAsync(stream, new { Field1 = "Value1", Field2 = "Value2" });
            });

            var client = new HttpClient();

            Console.WriteLine("Before http request");

            var response = await client.PostAsync("https://google.com", content);

            Console.WriteLine("After http request");

"After http request" is never printed. Disposal never happen in that approach.

@pranavkm pranavkm reopened this Jan 13, 2022
@pranavkm
Copy link
Contributor

@lichutin-st you're right, PushStreamContent does wait for the stream to be closed / disposed. I can look at updating the documentation here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants