Null check bug fixes and tweak accessibility of properties on TransportResponse

[stevejgordon]

DefaultResponseFactory was throwing if the response stream was null. This can occur when an exception is thrown when sending the request (e.g., HttpRequestException), for example, when the HttpClient cannot connect to the endpoint. Rather than throwing a null exception here, we still want to return a response with the original exception attached.

In StreamResponse, we must safety-check that any linked disposables are not null before attempting to dispose of them.

The final change in TransportResponse is a tweak for the ingest work. The BulkStreamingResponse was initially derived from the StreamResponse to share behaviour. However, this causes the Body property (stream) to be present on the derived type. As we are handling stream reading internally, this is unnecessary and could produce weird behaviour if the consumer tries to access the stream directly. Instead, BulkStreamingResponse derives directly from TransportResponse, overriding LeaveOpen and handling LinkedDisposables in its own Dispose method.

This means we could potentially seal StreamResponse again. However, it might still be helpful for consumers to derive responses from this for advanced scenarios, with the base class doing the right thing during disposal. I am open to thoughts on whether that's likely to happen. @flobernd, were you deriving from this in the client?

[flobernd]

This means we could potentially seal StreamResponse again. However, it might still be helpful for consumers to derive responses from this for advanced scenarios, with the base class doing the right thing during disposal. I am open to thoughts on whether that's likely to happen. @flobernd, were you deriving from this in the client?

I'm using a dedicated ResponseBuilder for the EsqlQueryResponse which is currently the only response type that access the raw stream, so sealing StreamResponse again would not be a breaking change for me.

I think we should seal it. A custom ResponseBuilder is required anyways to correctly instantiate the derived type. We could introduce an abstract StreamResponseBase that takes Stream in its constructor and derive StreamResponse from that. The base class should implement IDisposable and is intended to derive from, whereas StreamResponse itself remains as a quick solution to get access to the raw stream.

[stevejgordon]

@flobernd, that makes sense. I'll amend this PR to include that change.