jimmywarting / native-file-system-adapter

File system, based on the spec reference implementation
https://jimmywarting.github.io/native-file-system-adapter/example/test.html
MIT License
484 stars 43 forks source link

Service worker initial back pressure blocking downloads #49

Closed jason-graham closed 1 year ago

jason-graham commented 1 year ago

First off, great job on this library! Fills in the browser stream api gaps. I'm leveraging this library to support browsers that do not provide a great file api user experience or have no support at all for streaming (saving) data to the client. I'm using the example service worker, calling the showSaveFilePicker and piping a stream to the writable like so:

  const encoder = new TextEncoderStream();

  const fileHandle = await showSaveFilePicker({
    _preferPolyfill: true,
    suggestedName: this.suggestedFileName,
    types: [{accept: {"text/csv": [".csv"]}}],
    excludeAcceptAllOption: true,
  });

  await this.readable
    .pipeThrough(this.transform)
    .pipeThrough(encoder)
    .pipeTo(await fileHandle.createWritable());

When the above code runs, the iframe initiated request to download waits indefinitely. I worked backwards to find the downloader adaptor has an initial unresolved readyPromise that waits for an PULL request from the service worker, but the service worker only sends a PULL request when it gets a WRITE message and the downloader only sends a WRITE request when it gets a PULL request. Updating the start function of the service worker to this resolved the deadlock:

  start(controller) {
    this.controller = controller;
    //I think we need this?
    this.port.postMessage({ type: PULL });
  }

Are my assumptions/findings correct or did I miss something? I can open a pull request if this looks valid.

jimmywarting commented 1 year ago

Hmm 🤔 if you found a bug and you think that you have found the right solution then a PR is very much appreciated. Haven't tested this out myself in a long while if it work like it should 😅 May also have experimental support for transferable ReadableStream (that makes the postMessage fallback a bit obsolete)

Maybe while you are at it we could maybe switch out all private fields starting with _ to use private class fields/methods by using # instead. Browser support seems to have matured enough that all green browser now supports it.

jimmywarting commented 1 year ago

Gosh, i took an extra look at my code and i see that it isn't using the same kind of tech as my streamsaver.js lib where i transfer a ReadableStream over to the service worker...

jimmywarting commented 1 year ago

We should try to use Transferable ReadableStream whenever it's possible to do so (instead of keep sending postMessages to keep the service worker alive)

Umm, 🤔 then again... i'm a bit uncertain if Transferable ReadableStream is a bit slower? one of the initial goal is for the chunks to be transferable; instead they are cloned/copied: https://github.com/whatwg/streams/blob/main/transferable-streams-explainer.md#non-goals

Here i'm using a transferable list: https://github.com/jimmywarting/native-file-system-adapter/blob/d43ad841581c2cc3ce47bbd1e8f11950ebdff027/src/adapters/downloader.js#L116-L121

so i assume that it's faster than using TransferableStream... transferable might also be safer as service worker are quite picky about having a hard code lifetime duration

jason-graham commented 1 year ago

Very interesting -- seems like a transferred ReadableStream would greatly simplify the service worker.

jimmywarting commented 1 year ago

It dose, don't remember why i didn't use it when i initially write this when i already knew about transferable ReadableStream

maybe b/c no other than chrome supported it, or maybe b/c it was only avalible behind a experimental flag. or that chunks are copied instead of transfered those making it slower than traditional postMessage

jimmywarting commented 1 year ago

just tested out if FF support transferable streams and it dose

onmessage = console.log
rs = new ReadableStream()
postMessage(rs, '*', [rs])

safari did not support it. https://bugs.webkit.org/show_bug.cgi?id=215485

jimmywarting commented 1 year ago

then again, safari isn't using the same download tech as other as it's incompatible of downloading a file generated by the service worker. there is a long discussion about the issue here: https://github.com/jimmywarting/StreamSaver.js/issues/69

but recent announcement says it can download stuff from v15.5

jimmywarting commented 1 year ago

maybe we can remove the safari specific hack too...? what do u think? Worth trying out

psi-4ward commented 1 year ago

Bit off topic but probably not worth an separate issue.

Can one quickly outline why we need to use an sw at all?

jimmywarting commented 1 year ago

Can one quickly outline why we need to use an sw at all?

(A answer to a question like that from me is rarely never short) - sry, have had to explain it quite a bunch for others within streamsaver.js lib


if showSaveFilePicker isn't natively available then there is no other way to save a ReadableStreams directly to the disc.

The only other solution is to consume the stream and assemble a Blob in memory and use <a href="blob:url..." download="name"> (this work okey for small tiny files that are no larger then a few KiB / MiB that can be downloaded quick)

There are 2 problems with creating a Blob and then saving it

  1. The blob needs to be constructed in memory and can't old large GB of data without crashing the browser
  2. Consuming a large stream into a blob can take time. only once it's finish are you able to trigger downloadLink.click() to initiate the download. by the time you got a Blob you may have lost the isTrusted flag. that is only true for a short duration. So triggering the click / download on a link afterwards can prevent automatic download

A service worker on the other hand can emulate how a traditional server saves a file. you create a link, you start downloading right away (without having all data ready at once) and then you can write data little by little at the time as data flows in directly to the disc. or more precisely send data (or a ReadableStream) to the service worker and emulate how a server saves file by responding with a own Response constructed with a ReadableStream

The other benefit is that you get from just emulating how a backend server works with the help of Service Worker are that you get a

this is part of why i decided to add the option _preferPolyfill cuz some ppl want to use service worker option instead of the native method.

(hope that you got it)

psi-4ward commented 1 year ago

Brilliant! Thanks a lot!

jimmywarting commented 1 year ago

also the writable stream also accept all kind of data you throw at it, typed arrays, string, blob/files & arraybuffer

but if you are only saving one file/blob like this:

  const encoder = new TextEncoderStream();

  const fileHandle = await showSaveFilePicker({
    _preferPolyfill: true,
    suggestedName: this.suggestedFileName,
    types: [{accept: {"text/csv": [".csv"]}}],
    excludeAcceptAllOption: true,
  });

  const writable = await fileHandle.createWritable()
  writable.write(blob)
  writable.close()

then you already got a blob and doing this it's just better:

const a = document.createElement('a')
a.href = URL.createObjectURL(blob)
a.download = this.suggestedFileName
a.click()
URL.revokeObjectURL(a.href)

then you don't really need any extra lib or service worker to download anything.