Skip to content
This repository has been archived by the owner on Dec 18, 2018. It is now read-only.

ThrowResponseAlreadyStartedException while setting StatusCode in a Controller method #2667

Closed
ghost opened this issue Jun 14, 2018 · 16 comments
Milestone

Comments

@ghost
Copy link

ghost commented Jun 14, 2018

My use case is pretty simple, I have a method from an asp.net core 2.1 controller that is call for a given post request.

All I want to do is write some binary data in the Response.Body and set the Response.StatusCode.

No matter what I try it throws a ThrowResponseAlreadyStartedException stating:

StatusCode cannot be set because the response has already started.

All I want to do is something like this:

Response.ContentType = "application/octet-stream";
Response.Body.Write(someData, 0, someData.Length);
Response.StatusCode = (int) HttpStatusCode.OK;

Maybe I'm not using the right way to write binary data in the Response's body but that's the only one I came up with. Changing the signature of my method to return Task<ActionResult<byte[]>> won't do the trick to return binary data.

@davidfowl
Copy link
Member

Set the status code before writing. Writing the response will start it. That’s what the rrror message is trying to say. Maybe it’s not clear enough.

@benaadams
Copy link
Contributor

You have to set the status code prior to writing to the body

Switch the last two lines

Response.ContentType = "application/octet-stream";
Response.StatusCode = (int) HttpStatusCode.OK;
Response.Body.Write(someData, 0, someData.Length);

@benaadams
Copy link
Contributor

Maybe go async as well 😉

await Response.Body.WriteAsync(someData, 0, someData.Length);

@ghost
Copy link
Author

ghost commented Jun 14, 2018

Thing is in this specific case, I need to write to the body first, then the StatusCode depends on something else.
For the async, I wasn't sure it worth as it's a small protobuf buffer...

@ghost
Copy link
Author

ghost commented Jun 14, 2018

and by the way, if I return the data on the method return like this:
return Ok(myBinaryArray)

I get this in the debug console:

warn: Microsoft.AspNetCore.Mvc.Infrastructure.DefaultOutputFormatterSelector[1]
      No output formatter was found for content type 'application/octet-stream' to write the response.
VoxelService: Warning: No output formatter was found for content type 'application/octet-stream' to write the response.
warn: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor[1]
      No output formatter was found for content type 'application/octet-stream' to write the response.

But I've added this and no change....

services.AddMvc(options =>
            {
                options.OutputFormatters.Add(new StreamOutputFormatter());
            })

@ghost
Copy link
Author

ghost commented Jun 14, 2018

@benaadams WriteAsync doesn't change anything... I'll try to take a look at the implem behind but so far I don't understand the logic behind this. Why can't I set the body's content then set the Status Code?
Thanks for helping

@davidfowl
Copy link
Member

The response body isn’t buffered m. When you write it will be written (with headers and the start line) to the client.

@Tratcher Tratcher added this to the Discussions milestone Jun 14, 2018
@benaadams
Copy link
Contributor

@nockawa the headers; of which status code is one, occur in the output before the response body, so they can't be written after the body is written.

You could buffer the output before sending it if you expect that the status code could change while processing the request; then set the status code, followed by the buffered response.

e.g. new up and write to a MemoryStream

then

var memStream = new MemoryStream();
// Write data buffering it
memStream.Write(someData, 0, someData.Length);

// Set headers
Response.ContentType = "application/octet-stream";
Response.StatusCode = (int) HttpStatusCode.OK;
Response.ContentLength = memStream.Length; // might as well...

// Write buffered data to body
await memStream.CopyToAsync(Response.Body);

@ghost
Copy link
Author

ghost commented Jun 15, 2018

@benaadams yes, it totally makes sense, I thought about this afterward. Initially, I thought it wouldn't matter because I was in a Controller and not a middleware, but afterward I realized it was the only way to achieve great performances to process everything in order.

@davidfowl is that normal that I can't return a byte[] with a body content type of application/octet-stream and expect this to work?

Because the whole initial 'issue' is because of this. I initially wanted to mark the response content as octet-stream, set the Status Code and return a byte[] for the data but I never got it worked, so I tried different approach with the Body's stream.

@davidfowl
Copy link
Member

Seems like an mvc question. You’d need to show the original code you wrote.

There’s nothing special here at the kestrel level.

@ghost
Copy link
Author

ghost commented Jun 15, 2018

Seems like an mvc question. You’d need to show the original code you wrote.

Do you want me to post an issue on the corefx repo?

In the meantime, here's the code:

[Route("cell/{constructId}/{x}/{y}/{z}/{h}/version")]
[HttpGet]
public async Task<ActionResult<byte[]>> GetCellHashAndVersion(ulong constructId, long x, long y, long z, byte h)
{
	// Return the Cell Model, we should always get on, whether the cell is new or not
	var cell = await _service.GetCellAsync(constructId, x, y, z, h);

	// Setup the response
	Response.ContentType = "application/octet-stream";
	using (var ms = new MemoryStream(2048))
	{
		cell.ToPBCellVersion(ms);
		return Ok(ms.ToArray());
	}
}

Doing this display this warning message :

warn: Microsoft.AspNetCore.Mvc.Infrastructure.DefaultOutputFormatterSelector[1]
      No output formatter was found for content type 'application/octet-stream' to write the response.
VoxelService: Warning: No output formatter was found for content type 'application/octet-stream' to write the response.
warn: Microsoft.AspNetCore.Mvc.Infrastructure.ObjectResultExecutor[1]
      No output formatter was found for content type 'application/octet-stream' to write the response.

With a Status Code of 406 (I believe this is due to the OutputFormatter not willing to deal with byte[])
Again, I've added this formatter options.OutputFormatters.Add(new StreamOutputFormatter());

@davidfowl
Copy link
Member

@nockawa for future reference MVC issues go here (https://github.com/aspnet/Mvc).

Seems like MVC doesn't support that content type, the StreamOutputFormatter is for Streams, not byte[]. https://github.com/aspnet/Mvc/blob/6df28ef09a9d50e4601fc832a787b6d5c6635dc9/src/Microsoft.AspNetCore.Mvc.Core/Formatters/StreamOutputFormatter.cs#L24

public class HomeController : Controller
{
    [HttpGet("/bin")]
    [Produces("application/octet-stream")]
    public byte[] GetBin()
    {
        return Encoding.UTF8.GetBytes("Hello World");
    }

    [HttpGet("/bin2")]
    public Task GetBinRaw()
    {
        Response.ContentType = "application/octet-stream";
        var buffer = Encoding.UTF8.GetBytes("Hello World");
        return Response.Body.WriteAsync(buffer, 0, buffer.Length);
    }

    [HttpGet("/bin3")]
    [Produces("application/octet-stream")]
    public Stream GetBinStream()
    {
        return new MemoryStream(Encoding.UTF8.GetBytes("Hello World"));
    }
}

Seems like MVC just doesn't support returning a byte[] directly. The other 2 methods work.

/cc @rynowak for more info

@davidfowl
Copy link
Member

@nockawa if you want to avoid allocations, you can write directly to the response stream.

@ghost
Copy link
Author

ghost commented Jun 16, 2018

@davidfowl thanks for all the help, I think I'll manage from there.
I still do think that returning byte[] should automatically translate to set the ContentType as application/octet-stream and copy the result in the internal Stream. I assume it's what is done for text counterpart.

@ghost ghost closed this as completed Jun 16, 2018
@davidfowl
Copy link
Member

@davidfowl thanks for all the help, I think I'll manage from there.
I still do think that returning byte[] should automatically translate to set the ContentType as application/octet-stream and copy the result in the internal Stream. I assume it's what is done for text counterpart.

Agreed. File an issue on MVC. I'm not sure why we don't support byte[] but Stream works.

@ghost
Copy link
Author

ghost commented Jun 16, 2018

that's what I'm doing right now! :) I just make sure I give all the details I can

This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants