firebase / firebase-js-sdk

Firebase Javascript SDK
https://firebase.google.com/docs/web/setup
Other
4.76k stars 874 forks source link

Use of XHR prevents Storage upload in Service Workers #902

Open GabiAxel opened 6 years ago

GabiAxel commented 6 years ago

[REQUIRED] Describe your environment

[REQUIRED] Describe the problem

Storage upload functionality uses XhrIo, which uses XHR. XHR is not available in Service Workers, so it's not possible to upload files to storage in Service Workers with the current JS SDK.

I propose replacing the use of XHR with Fetch (or Websockets?) or at least support both and provide a way to switch to Fetch.

Use case: resumable upload of large files over slow networks which will not break when the user navigates away or closes the browser window.

Steps to reproduce:

In a Service Worker, attempt to upload something

Relevant Code:

self.importScripts('https://unpkg.com/firebase@5.0.4/firebase.js')
...
self.firebase.storage().ref().child(filename).put(file)

throws Uncaught ReferenceError: XMLHttpRequest is not defined

GabiAxel commented 6 years ago

I overcame this by polyfilling XHR based on how it's used by NetworkXhrIo with Fetch, and I got Firebase Storage upload to work in Service Worker.

There is one missing piece: Fetch doesn't provide a way to observe a a request progress, so Firebase upload task only fires state_changed when a chunk finishes uploading, which can be noticeable for large files (new chunk size is doubled after a previous successful chunk upload, so there are fewer progress updates). I suppose this could be resolved when Background Fetch is implemented.

Here is the XHR polyfill snippet:

importScripts('https://unpkg.com/eventemitter3@3.1.0/umd/eventemitter3.min.js')

class XMLHttpRequest {

  constructor() {
    this.eventEmitter = new EventEmitter3()
    this.requestHeaders = {}
  }

  addEventListener(eventName, listener) {
    this.eventEmitter.on(eventName, listener)
  }

  removeEventListener(eventName, listener) {
    this.eventEmitter.off(eventName, listener)
  }

  open(method, url) {
    this.method = method
    this.url = url
  }

  setRequestHeader(key, value) {
    this.requestHeaders[key] = value
  }

  send(body) {
    this.controller = new AbortController()
    this.controller.signal.addEventListener('abort', () =>
      this.eventEmitter.emit('abort')
    )
    fetch(this.url, {
      method: this.method,
      headers: this.requestHeaders,
      body,
      signal: this.controller.signal
    }).then(response => {
      this.status = response.status
      this.responseHeaders = response.headers
      return response.text()
    }).then(responseText => {
      this.responseText = responseText
      this.eventEmitter.emit('load')
    }).catch(() =>
      this.eventEmitter.emit('error')
    )
  }

  abort() {
    this.controller.abort()
  }

  getResponseHeader(key) {
    return this.responseHeaders.get(key)
  }
}
jimmywarting commented 4 years ago

unfortunately @GabiAxel polyfill didn't work for me...

can the fetchxmlhttpfactory.js be plugged into XhrIo somehow?

liqwid commented 2 years ago

For anyone looking here, fixed the problem with file upload (tested in chrome extension v3 service worker)

The issue is Firebase's Connection expects the response string in response field of xhr which is missing. Fixed by adding this.response = responseText right before this.responseText = responseText. Not sure if responseText is used by Connection, left it there.

Full code:

importScripts('https://unpkg.com/eventemitter3@3.1.0/umd/eventemitter3.min.js')

class XMLHttpRequest {

  constructor() {
    this.eventEmitter = new EventEmitter3()
    this.requestHeaders = {}
  }

  addEventListener(eventName, listener) {
    this.eventEmitter.on(eventName, listener)
  }

  removeEventListener(eventName, listener) {
    this.eventEmitter.off(eventName, listener)
  }

  open(method, url) {
    this.method = method
    this.url = url
  }

  setRequestHeader(key, value) {
    this.requestHeaders[key] = value
  }

  send(body) {
    this.controller = new AbortController()
    this.controller.signal.addEventListener('abort', () =>
      this.eventEmitter.emit('abort')
    )
    fetch(this.url, {
      method: this.method,
      headers: this.requestHeaders,
      body,
      signal: this.controller.signal
    }).then(response => {
      this.status = response.status
      this.responseHeaders = response.headers
      return response.text()
    }).then(responseText => {
      this.response = responseText
      this.responseText = responseText
      this.eventEmitter.emit('load')
    }).catch(() =>
      this.eventEmitter.emit('error')
    )
  }

  abort() {
    this.controller.abort()
  }

  getResponseHeader(key) {
    return this.responseHeaders.get(key)
  }
}
hsubox76 commented 11 months ago

Going through old issues and closing the ones that seem obsolete. This one seems like it might still be an active bug as we still do use xhr in Storage and Connection still queries xhr.response, so I'll leave it open.

jimmywarting commented 11 months ago

Should be switching to using fetch api instead. IMO

dojoVader commented 2 months ago

This is an active bug, should be fixed and converted to fetch for MV3 in service workers.

Edited: XmlHttpRequest-shim worked for us.

hsubox76 commented 2 months ago

The complicating factor with fetch is that it does not support upload progress, which is currently a feature of the Storage SDK.

We do have a Node bundle for Storage that uses fetch but I'm not sure if other Node-specific features might cause errors when using it in a service worker. If you want to try it out (and you don't need upload progress), you can configure your bundler (webpack, rollup, etc) to prefer the Node bundle for @firebase/storage, if you are able to do that (using mainFields or conditionNames), and see if it works for you.

dojoVader commented 2 months ago

@hsubox76 thanks I understand, because for our purpose we don't need it for upload, we simply just did a polyfill to modify XHR to fetch and that's working for us.

Also thanks I had no idea Fetch didn't support progress upload.