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

Construct 3 - Cloud Uploading Progress Broken (Percentage Message) #565

Closed
TheRealDannyyy opened this issue Jun 28, 2017 · 16 comments
Closed
Assignees
Milestone

Comments

@TheRealDannyyy
Copy link

TheRealDannyyy commented Jun 28, 2017

Problem description

This is a quick one and probably not even caused by Construct 3 itself.
The uploading percentage found on the bottom-left isn't working, it stays at 0% even though the syncing process is working fine.

upload progress

Steps to reproduce

  1. Open any project
  2. Save to cloud (in my case Google Drive)
  3. Notice that the progress stays at 0%

Observed result

Construct 3 displays the cloud upload percentage but it stays at 0%.

Expected result

Construct 3 should display the progress from 0 to 100%.

Affected browsers

  • Chrome: yes
  • Firefox: yes/no
  • Edge: yes/no
  • Safari: yes/no

System details

  • Chrome, latest stable. (Version 59.0.3071.115)
@TheRealDannyyy TheRealDannyyy changed the title Construct 3 - Cloud Uploading Progress Broken Construct 3 - Cloud Uploading Progress Broken (Percentage Message) Jun 28, 2017
@AshleyScirra
Copy link
Member

Assigning to Iain, but IIRC not all cloud services actually give a progress, so it may not be fixable for every service...

@TheRealDannyyy
Copy link
Author

Yeah I knew that this was probably not a C3 issue. Either way I think it's a good thing if we keep this one open until all workarounds have been tested out or perhaps Google updates their cloud API.

@JeFawk
Copy link

JeFawk commented Jul 2, 2017

I'm using dropbox and it shows 0% all the time as well.

@shortercode
Copy link

I can confirm this is working in development and not release. I've checked through and can't find any build related issues, the progress event seems to not be firing.

I think this might be related to the service worker intercepting the networking request w3c ServiceWorker issues 1141 and completing it with the fetch API.

For reference:

  • all upload methods should provide progress events
  • download methods using xhr.onprogress and upload methods are using xhr.upload.onprogress
  • download methods are firing progress events, but upload methods are not
  • in the development environment upload and download are working
  • event handlers are being bound correctly in both cases
  • xhr supports progress events for both upload and download
  • fetch doesn't support progress events
  • in release all network requests from the tab are routed via the service worker

I haven't been able to find a specific chrome bug related to this, just the github issue I linked above. By the sounds of it some slight tweaks to the service worker may solve the issue? That's more @AshleyScirra's realm though.

@TheRealDannyyy
Copy link
Author

Alright, seems the like SW's are being a pain again.
This isn't a high priority right now so, @AshleyScirra feel free to fix this whenever you got the time to do it.

Thanks for the heads up @shortercode.

@AshleyScirra
Copy link
Member

AshleyScirra commented Jul 3, 2017

This sounds like the previous issue I was having where downloading example projects also failed to track progress. I never got to the bottom of that (it was another thing that changed in production), but it sounds like from that w3c thread that it's not really well supported yet to have progress events through a Service Worker. Not sure I can be of much more help than @shortercode . This looks like an appropriate time to add the "difficult" tag...

@shortercode
Copy link

Little red label of doom...

Status on progress events is not good, they seem to have been tied up in the cancel-able promises proposal. Which was around for 2 years before ironically being cancelled.

The new proposal still looks pretty flaky, but it might arrive sometime soon(ish) if observables make it into the spec. I've tried looking into non serviceworker workarounds and I can only find ones for downloads, so that's pretty useless.

Best suggestions I have:

  1. When the serviceworker receives a no-cache entry just bail instead of using respondWith( fetch(request) ) this triggers the default behaviour instead, so the progress event starts working again ( tested )
  2. Complain at the chrome team ( might do this anyway )

@shortercode
Copy link

shortercode commented Jul 18, 2017

Assigning to @AshleyScirra for a possible service worker refactor

@AshleyScirra
Copy link
Member

I don't think I'll actually get round to a SW refactor any time soon, and on further reflection your patch is probably a pretty straightforward hack to apply until that refactor does happen. So I've changed my mind, @shortercode go ahead and apply your patch to the SW.

@shortercode
Copy link

Okay I've merged the change in, should appear in r46

@shortercode shortercode added this to the r46 milestone Jul 19, 2017
@AshleyScirra
Copy link
Member

The root SW (for editor.construct.net) needs to be updated server-side so note this may not actually sync up with r46. Also there's probably going to be caching issues, I'm not sure when browsers will pick up an updated SW script, especially with CloudFlare's caching...

@shortercode
Copy link

I'm pretty sure its the version specific service worker that catches the request, not the root service worker. So we might be in business with a release.

@AshleyScirra
Copy link
Member

Our SW setup is kind of a mess, because the version specific SW does not actually intercept any fetch requests at all :P its scope is 'eclipsed' by the root one, which always handles everything on editor.construct.net. The version-specific one only serves to fire an install event (which caches that version), or handle fetches if you directly navigate to the version (e.g. editor.construct.net/r45). None of this is what we intended, but it sort of ends up working anyway by accident.

@TheRealDannyyy
Copy link
Author

No problem if it takes a couple of releases in order to sync up. This is a visual issue and nothing engine breaking so any kind of minor delay is alright in my book.

@shortercode
Copy link

Seems to be working for me in r46, look okay to you @TheRealDannyyy ?

@TheRealDannyyy
Copy link
Author

@shortercode it indeed does work now. Thanks a lot for the quick fix!

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

No branches or pull requests

4 participants