Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Service Worker makes AJAX Progress Listener not Working #1141

Open
albertsylvester opened this issue May 16, 2017 · 13 comments
Open

Service Worker makes AJAX Progress Listener not Working #1141

albertsylvester opened this issue May 16, 2017 · 13 comments

Comments

@albertsylvester
Copy link

albertsylvester commented May 16, 2017

Hi everyone,

With Service Worker (SW) installed on a web apps, it caused "AJAX Progress Listener" not working.
Any idea?

Tested in Chrome Desktop, Chrome Mobile v 58.0, Firefox Desktop v 52.0.2, Firefox Android v 53.0.2
It's working
Thanks in advance

The Service Worker

if ('serviceWorker' in navigator && (window.location.protocol === 'https:')) {
        navigator.serviceWorker.register('/service-worker.js')
        .then(function(registration) {
          //Checks the server for an updated version of the service worker without consulting caches.
          //registration.update();
          
          // updatefound is fired if service-worker.js changes.
          registration.onupdatefound = function() {
            // updatefound is also fired the very first time the SW is installed,
            // and there's no need to prompt for a reload at that point.
            // So check here to see if the page is already controlled,
            // i.e. whether there's an existing service worker.
            if (navigator.serviceWorker.controller) {
              // The updatefound event implies that registration.installing is set:
              // https://slightlyoff.github.io/ServiceWorker/spec/service_worker/index.html#service-worker-container-updatefound-event
              var installingWorker = registration.installing;

              installingWorker.onstatechange = function() {
                switch (installingWorker.state) {
                  case 'installed':
                    // At this point, the old content will have been purged and the
                    // fresh content will have been added to the cache.
                    // It's the perfect time to display a "New content is
                    // available; please refresh." message in the page's interface.
                    console.warn('New content is available, please refresh the page');
                    break;

                  case 'redundant':
                    throw new Error('The installing ' +
                                    'service worker became redundant.');

                  default:
                    // Ignore
                }
              };
            }
          };
        }).catch(function(e) {
          console.error('Error during service worker registration:', e);
        });
      }

And, the Ajax Uploader

function uploadPic(){
        var file = document.getElementById('fileInput').files[0];
        var fd = new FormData();
        fd.append("file", file);

        var xhr = new XMLHttpRequest();

        //attach listener before posting
        xhr.upload.onprogress = updateProgressBar;

        xhr.onreadystatechange = function() {//Call a function when the state changes.
            if(xhr.readyState == 4 && xhr.status == 200) {
                console.log(xhr.responseText);
            }
        }

        xhr.open("POST", "https://www.mysite.com/upload.php", true);
        xhr.send(fd);
      }

      function updateProgressBar(e){
        if (e.lengthComputable) {
          var percentComplete = (e.loaded / e.total) * 100;
          console.log(percentComplete + '% uploaded', e);
        }
      }
@wanderview
Copy link
Member

This is about progress on an upload body, right?

The progress may not be right if the service worker script clones or reads the FetchEvent.request's body. Are you doing that? Can you post your fetch event handler?

Besides that, I am willing to bet service worker implementations have bugs related to this. In gecko I am fairly sure we don't pipe back progress from the network socket, through the SW, back to the original XHR. In theory this could be possible if you do a pass-through like fetch(event.request).

@wanderview
Copy link
Member

In gecko I think progress should work if you avoid calling respondWith() for these requests in your service worker.

@albertsylvester
Copy link
Author

albertsylvester commented May 16, 2017

Hi wanderview,

thanks for the answer. Yeah, I think I know the answer. It is because of the fetch API in my SW.

service-worker.js (Fetch Section)

self.addEventListener('fetch', function(event) {
  console.log('Handling fetch event for', event.request.url, version);

  event.respondWith(
    caches.match(event.request).then(function(response) {
      if (response) {
        //console.log('Found response in cache:', response);
        return response;
      }
      //console.log('No response found in cache. About to fetch from network...', event.request.url);

      return fetch(event.request).then(function(response) {
        //console.log('Response from network is:', response);
        return response;
      }).catch(function(error) {
        console.error('Fetching failed:', error);
        throw error;
      });
    })
  );
});

Here's a reference: JakeChampion/fetch#89

@wanderview
Copy link
Member

@albertsylvester Since Cache API never stores "POST" requests, you could put this at the top of your fetch event handler to avoid the respondWith():

if (event.request.method === 'POST') {
  return;
}

I'm not sure we can make the pass-through fetch(event.request) work for progress until we fix progress in the Fetch API spec.

@albertsylvester
Copy link
Author

Thanks @wanderview. You are awesome. Why do you want to avoid respondWith()?
Thank you

@wanderview
Copy link
Member

Why do you want to avoid respondWith()?

It allows the network request to complete just like it would without the service worker (in theory).

@wanderview
Copy link
Member

Sorry, I think maybe we should keep this open to track the problem with progress when using a pass-through service worker.

@wanderview wanderview reopened this May 16, 2017
@albertsylvester
Copy link
Author

Ok @wanderview . Thanks

@jakearchibald
Copy link
Contributor

@wanderview which part of the spec is causing this to break currently?

@wanderview
Copy link
Member

Uh, fetch API doesn't have any progress API yet? Don't we need FetchObserver for that? Until we have that we can't really propagate progress through the respondWith() accurately. The best you can do is use a ReadableStream for the upload and assume when its read its been uploaded, but re-buffering can be taking place.

@jakearchibald
Copy link
Contributor

I thought, because of the way the stream was adopted, https://fetch.spec.whatwg.org/#concept-body-transmitted would still update, but I need to take a closer look. Fetch observing would certainly make this clearer.

@wanderview
Copy link
Member

I wasn't aware of the "transmitted bytes" language in fetch upload stream spec. I guess it might be adequate for the progress issue, but it also kind of seems like a de-opt for upload throughput to me. It doesn't really match actual browser implementation (at least ours) very well.

@isocroft
Copy link

@jakearchibald would it be possible to create a polyfill using the stream object for upload progress within/for the Fetch API ??

From this: https://github.com/yutakahirano/fetch-with-streams/

It seems a bit complicated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants