Skip to content

fix(storage): snapshotChanges should return a success snapshot - #2704

Merged
jamesdaniels merged 4 commits into
masterfrom
from_running_task
Dec 3, 2020
Merged

jamesdaniels merged 4 commits into
masterfrom
from_running_task

Conversation

@jamesdaniels

@jamesdaniels jamesdaniels commented Dec 3, 2020 •

Copy link
Copy Markdown
Contributor

Ugh this was still broken. task.snapshot and task.on('state_changed', ...) never return the "last" snapshot, have to use the promise form of task to get at it.

  • Works on tasks that are already running
  • Now emits a final success snapshot before completion
  • Emits a filterable canceled or error snapshot prior to a throw There appears to be a JS SDK bug blocking this 😢 in theory it should work in a future release
  • Added tests for canceling, pausing, and resuming

@jamesdaniels jamesdaniels changed the title fix(storage): fromTask on a running task should work as expected fix(storage): snapshotChanges should return a complete task snapshot Dec 3, 2020
@jamesdaniels jamesdaniels changed the title fix(storage): snapshotChanges should return a complete task snapshot fix(storage): snapshotChanges should return a success task snapshot Dec 3, 2020
@jamesdaniels

jamesdaniels commented Dec 3, 2020 •

Copy link
Copy Markdown
Contributor Author

It appears I'm going in circles here. I believe this is a JS SDK bug firebase/firebase-js-sdk#4158

I've put in enough work arounds to have things working decently IMO but developers still can't filter errors before they throw...

@jamesdaniels jamesdaniels changed the title fix(storage): snapshotChanges should return a success task snapshot fix(storage): snapshotChanges should return a success snapshot Dec 3, 2020
@jamesdaniels
jamesdaniels merged commit 972aa85 into master Dec 3, 2020
@jamesdaniels
jamesdaniels deleted the from_running_task branch December 3, 2020 22:17
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants