Streaming file support

[jivank]

I am wondering if you will drop support for httpbeast for netkit?

I have a pull request that will stream large files, its a simple change but it won't fix the memory leak with httpbeast. Additionally the content-length is sent twice because it appears httpbeast is loading the body data into its own buffer before sending it, so I corrected this with when defined, but the content length shows as 0.

On macOS it will serve a 600mb and the executable will use 4.9MB memory afterwards.

Please let me know if you would like me to create a pull request.
context.nim

  else:
    when defined(windows) or defined(usestd):
      ctx.response.setHeader("Content-Length", $contentLength)
    await ctx.respond(Http200, "", headers)
    var
      file = openAsync(filePath, fmRead)

    while true:
      let value = await file.read(4096)
      if value.len > 0:
        await ctx.send(value)
      else:
        break

    file.close()
    ctx.handled = true

[bung87]

it's acceptable, the use case limit to download file , and set a proper header tell browser

[ringabout]

@jivank
Look at this issue, I think it should be fixed in httpbeast.
asynchttpserver has the same issue before v1.2.
dom96/jester#241

When netkit is stable, I will change to it.

[ringabout]

I think the previous way should work too. Do you have issues with streaming file when you use asynchttpserver and the previous way?

[jivank]

@xflywind

Are you referring to
when defined(windows) or defined(usestd):
?

asynchttpserver will work without that line. Would you like to me to remove it? I only placed it there so that httpbeast would work but maybe httpbeast already was having an issue.

[ringabout]

@jivank
I mean this, use future stream to send file. It is almost the same way.

  else:
    # TODO stream
    ctx.response.setHeader("Content-Length", $contentLength)
    await ctx.respond(Http200, "", headers)
    var
      fileStream = newFutureStream[string]("staticFileResponse")
      file = openAsync(filePath, fmRead)

    await file.readToStream(fileStream)

    while true:
      let (hasValue, value) = await fileStream.read
      if hasValue:
        await ctx.send(value)
      else:
        break

    file.close()
    ctx.handled = true

https://github.com/nim-lang/Nim/blob/7800fa394f26e9a5a6cb30964ba005512199be58/lib/pure/asyncfile.nim#L518

[jivank]

The old method will use ~900MB of memory for the 600MB file. I think this is because readToStream will fully load the file into memory. Also I believe @dom96 mentioned that he needs to rewrite/redesign the streams library.

[ringabout]

@jivank
Alright, pr is welcome. But when defined is unnecessary because httpbeast is broken in sending multiple header fields.

[bung87]

close as #16

[dom96]

httpbeast could probably benefit from a proc which copies the contents of a file to its fd. There is a POSIX API call for this, if someone wants to implement it I will accept.

Please submit issues for these kinds of things if you haven't already. In particular to the Nim repo since I think the future stream implementation needs fixing.

[ringabout]

Now httpx backend supports streaming file in both windows and unix.