Throw new exception carrying $chunk when fwrite returns 0

[ostrolucky]

Fixes #51

I tried hard to write a test for simulating such write failure, but any trick I tried, stream_select will not work with those. Another option would be to use unqualified fwrite call, but I don't think that would be approved.

[ostrolucky]

friendly ping @kelunik @trowski I need this

[bwoebi]

I think this can be tested with a custom stream wrapper - I'll have to check. But the change itself is fine - thanks :-)

[ostrolucky]

Nope I tried that trick, that's how I discovered this amphp/amp#265

edit: btw thanks for merge (｡◕‿◕｡)

[kelunik]

I'm sorry, but I've reverted the commit. The reported information is incorrect in case write-splitting is used, see

byte-stream/lib/ResourceOutputStream.php

Lines 208 to 215 in 2120bdd

	if ($length - $written > self::LARGE_CHUNK_SIZE) {
	$chunks = \str_split($data, self::LARGE_CHUNK_SIZE);
	$data = \array_pop($chunks);
	foreach ($chunks as $chunk) {
	$this->writes->push([$chunk, $written, new Deferred]);
	$written += self::LARGE_CHUNK_SIZE;
	}
	}

.

Additionally, I still think giving out that information is misleading for the intended use case, because written bytes mostly don't let you know what the other side received. Those cases where that is the case, are not intended to use ResourceOutputStream AFAIK.

[ostrolucky]

I don't see how is information incorrect in case write splitting is used. It reports what part of chunk wasn't successful to be written. It doesn't matter that original chunk was splitted, it still returns correct information what data failed to be written

[kelunik]

@ostrolucky It will only return a part of the original chunk, not data based on the complete chunk.

[ostrolucky]

Yes, that's what I need. I need data that failed to be written, which might be just part of original chunk. I already have original chunk when passing it to write($chunk). What I don't have is information which part of chunk failed to be written because of internal splitting and retry logic.

I don't understand second part of your sentence though. Splitting is by definition based on complete chunk.

[ostrolucky]

Maybe $chunk confused you? It could use better naming. It's not original chunk, but last $data passed to fwrite

[trowski]

@ostrolucky When the chunk is split, the implementation was only returning one portion of the entire split chunk. Also any other chunks in the queue were not included. All remaining chunks in the write queue should be concatenated together if you want the entirety of what was not written.

As I said in the linked issue, I don't think this information is useful. It's misleading at best and potentially dangerous. A chunk reported as written is not at all guaranteed to have been received at the other end. I can set up a stream between two machines on my local network, unplug the receiver, and at least a few more chunks never received are reported as written. Additionally, attaching a raw byte-stream to an error can have security implications.

I don't exactly know what your use-case is, but I'm going to guess it's serving files over HTTP. Look into byte-range requests. The client should be telling you what bytes they need rather than guessing.

[ostrolucky]

A chunk reported as written is not at all guaranteed to have been received at the other end.

But chunk reported as unwritten is guaranteed to have not been received at the other end. This is what is this about. I don't care if client recieved all data, I care if all was sent. I can't do anything about data it cannot receive.

Ok I will see what can be done about extra chunk splitting.

[trowski]

How is that information useful though?

[ostrolucky]

This is prerequisite to do failure recovery. I want to write data failed to be written in one stream into another stream. What I must NOT have is overlapping data in one or other stream.

[kelunik]

@ostrolucky Which data format are you using? Most data formats aren't happy with arbitrary missing chunks in the middle.

[ostrolucky]

Binary