Deno: http/file_server close with Uncaught Error: Short write

[axetroy]

Not every time an exception is thrown, but most are

$ file_server
Compile https://deno.land/std/http/file_server.ts
HTTP server listening on http://0.0.0.0:4500/
[2019-12-10 02:51:41] "GET /bin/dvm_windows_amd64.tar.gz HTTP/1.1" 200
error: Uncaught Error: Short write
► https://deno.land/std/io/bufio.ts:458:18

458       this.err = new Error("Short write");
                     ^

    at flush (https://deno.land/std/io/bufio.ts:458:18)

[zhmushan]

works fine on my machine😢

HTTP server listening on http://0.0.0.0:4500/
[2019-12-10 04:08:06] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:06] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:06] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:06] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:07] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:07] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:07] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:07] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:07] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:07] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:08] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:08] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:08] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:08] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:08] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:08] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:09] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:09] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:09] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:09] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:09] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:10] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:10] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:10] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:10] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:10] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200
[2019-12-10 04:08:10] "GET /dvm_windows_amd64.tar.gz HTTP/1.1" 200

[kevinkassimo]

Relevant code:
https://github.com/denoland/deno/blob/c93ae0b05a4c4fe5b43a9bd2b6430637b17979d0/std/io/bufio.ts#L440-L463

It seems that error Short write would be thrown if
https://github.com/denoland/deno/blob/c93ae0b05a4c4fe5b43a9bd2b6430637b17979d0/std/io/bufio.ts#L446-L448
n is smaller than the total buffered bytes. (aka writing less than all). I wonder why this is considered an error to throw instead of just trying to write the remaining again...

[kevinkassimo]

The behavior seems to have been inherited from Go's bufio: https://golang.org/src/bufio/bufio.go?s=15189:15219#L595
And according to https://golang.org/pkg/io/#pkg-variables:

ErrShortWrite means that a write accepted fewer bytes than requested but failed to return an explicit error.

I have double checked write (2) description and it seems short write on *nix platforms is indeed indication some small problems (not so sure on windows).

However the problem I have with it here is that Go's File.Write is also implemented to include an error when writing less then requested, which is not reflect to our FileImpl.write, a conflict of behavior.

Short write does not seem to be too problematic and maybe trying again makes sense. If we still want to preserve the ShortWrite behavior as an error, IMO it might be a good idea to separate into BufWriter.tryFlush() which might throw short write error, and BufWriter.flush() which would repeatedly attempt write (repeat immediately or with some exponential backoff) until completed (or until some max-try budget is depleted). @ry on this?

Also I would imagine a reason why write fails to write exactly the size is due to dvm_windows_amd64.tar.gz large enough to be written in a single attempt. Maybe we should implement something for better streaming response.

[lengfangbing]

The behavior seems to have been inherited from Go's bufio: https://golang.org/src/bufio/bufio.go?s=15189:15219#L595
And according to https://golang.org/pkg/io/#pkg-variables:

ErrShortWrite means that a write accepted fewer bytes than requested but failed to return an explicit error.

I have double checked write (2) description and it seems short write on *nix platforms is indeed indication some small problems (not so sure on windows).

However the problem I have with it here is that Go's File.Write is also implemented to include an error when writing less then requested, which is not reflect to our FileImpl.write, a conflict of behavior.

Short write does not seem to be too problematic and maybe trying again makes sense. If we still want to preserve the ShortWrite behavior as an error, IMO it might be a good idea to separate into BufWriter.tryFlush() which might throw short write error, and BufWriter.flush() which would repeatedly attempt write (repeat immediately or with some exponential backoff) until completed (or until some max-try budget is depleted). @ry on this?

Also I would imagine a reason why write fails to write exactly the size is due to dvm_windows_amd64.tar.gz large enough to be written in a single attempt. Maybe we should implement something for better streaming response.

great~
so now we have a better streaming response?

[lideming]

This issue should have been fixed by #6269, which changed the behavoir of BufWriter.flush.

A single write call may fail to fully write the buffer when there is no enough space in the TCP sending window or the OS internal buffer. It will happen more frequently when the client pauses receiving data from the connection (e.g. when the browser is asking the user where to save the downloading file or the downloading task is paused).