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

Observe queue operations when using background sync #2044

Closed
bligny opened this issue Apr 24, 2019 · 11 comments
Closed

Observe queue operations when using background sync #2044

bligny opened this issue Apr 24, 2019 · 11 comments

Comments

@bligny
Copy link

bligny commented Apr 24, 2019

We are currently trying to reflect the actual queue contents into the main menu - using a counter representing pending updates (see picture):

To do so, we are using the brand new getAll() method on the Queue object.
Rather than showing this kind of information on a regular basis (typically upon application startup + every x seconds/minutes), we would like to be able to do it in real-time.

For this, we need to listen/observe the queue operations, especially:

  • each time a new request is enqueued
  • each time an existing request is dequeued (replayed)

Would it be possible to register some queue listeners (or trigger some events) in order to get notified after each successful pushRequest() and popRequest() so that I can respectively increase or decrease my applicative counter ?

IMHO, it's not a huge job...

@philipwalton
Copy link
Member

This was something I considered in the v4 rewrite (see the "open questions" section of first comment in the PR), but ultimately I opted to not have callbacks as we did in v3 because I thought most advanced users of background sync would be writing their own queue replay code (is this not true in your case?).

Here's a code sample from the comment that shows how to do the custom reply yourself by defining an onSync callback:

const queue = new workbox.backgroundSync.Queue('my-queue-name', {
  onSync: async (queue) => {
    let entry;
    while (entry = await this.shiftRequest()) {
      try {
        await fetch(entry.request);
        console.error('Replay successful for request', entry.request);
      } catch (error) {
        console.error('Replay failed for request', entry.request, error);

        // Put the entry back in the queue and re-throw the error:
        await this.unshiftRequest(entry);
        throw error;
      }
    }
    console.log('Replay complete!');
  }
});

Does this meet your needs?

@bligny
Copy link
Author

bligny commented Apr 25, 2019

Yes, I have read that post too. I fully understand (and respect) the motivations of this refactoring: the api is now much easier, at least for basic usage.
The main objection I have is that we basically have (only) two options:

  • either use the standard/default replay logic (eg queue.replayRequests() )
  • or implement from-scratch a custom replay logic

It's a bit a kind of 'all-or-nothing' choice.

Beside the standard or custom implementations, it would be really great to have a third option: extend/enrich the standard replay skeleton, and focus on extensions.

Of course, this kind of extension is trivial:

const queue = new workbox.backgroundSync.Queue('my-queue-name', {
  onSync: async (queue) => {
    // Run standard replay
    await queue.replayRequests();
    // Do custom *POST* processing (eg update my applicative counter)
    console.log('Extra feature');
  }
});

but it's limited to the POST processing of the WHOLE backlog.

How (without repeating the skeleton) to add custom processing INSIDE the loop for each INDIVIDUAL queue item ?
The only solution is to copy&paste original replayRequests, and then modifying this code:

const queue = new workbox.backgroundSync.Queue('my-queue-name', {
  onSync: async (queue) => {
    let entry;
    while (entry = await this.shiftRequest()) {
      try {
        await fetch(entry.request);
        // Do custom *INNER* processing (eg decrease my applicative counter by one)
       console.log('Extra feature for request', entry.request.url);
      } catch (error) {       
        await this.unshiftRequest(entry);
        throw error;
      }
    }
    console.log('Replay complete!');
  }
});

Yes it works, but it may sometimes be a pity to repeat 10 lines of source code in order to add one single line :-(

In particular, each time I want the use the standard replay logic + one very little additional feature, I always have to repeat the same skeleton (the loop with shiftRequest, the fetch, the unshift in case of error, etc...).
And if you fix a bug in the replayRequests() method - like you recently did by replacing fetch(entry.request) with fetch(entry.request.clone()) - my implementation will not benefit from this fix.

Does this meet your needs?

I'm afraid not. My use case is not fully covered.
I could indeed implement a custom replay logic in order to decrease my applicative counter, upon the 'sync' event, true.
But how do I handle the incrementation of my counter ?
How can I be notified when the bg-sync plugin is putting a new request into the queue ?

My 2 cents

Bernard.

@ThaDaVos
Copy link

I think @bligny is right, sometimes you just want to hook in without changing the logic itself, for example when adding a counter

@jeffposnick jeffposnick changed the title Background-Sync : listen for queue operations ? Observe queue operations when using background sync Nov 21, 2019
@maximilianoforlenza
Copy link

maximilianoforlenza commented Dec 10, 2019

Hi @jeffposnick ! How I can send the requests pending to the client?
I have this code:

addEventListener('fetch', (event) => {
  if (['POST, PUT, DELETE'].includes(event.request.method)) {
    const promiseChain = fetch(
      event.request.clone()
    ).catch(() => queue.pushRequest({request: event.request}));
    event.waitUntil(promiseChain);
  }
});

It's ok. Then I want to send these requests failed to the client using the getAll method.
Should I use postMessage?
Thanks!

@jeffposnick
Copy link
Contributor

CC: @philipwalton for guidance.

@realtebo
Copy link

I suggest this updated version: note the queue destrctured from param

onSync: async ({queue}) => {
      let entry;
      while ((entry = await queue.shiftRequest())) {
        try {
          await fetch(entry.request);
          console.error("Replay successful for request", entry.request);
        } catch (error) {
          console.error("Replay failed for request", entry.request, error);

          // Put the entry back in the queue and re-throw the error:
          await queue.unshiftRequest(entry);
          throw error;
        }
      }
      console.log("Replay complete!");
    }

@silvergravel
Copy link

I suggest this updated version: note the queue destrctured from param

onSync: async ({queue}) => {
      let entry;
      while ((entry = await queue.shiftRequest())) {
        try {
          await fetch(entry.request);
          console.error("Replay successful for request", entry.request);
        } catch (error) {
          console.error("Replay failed for request", entry.request, error);

          // Put the entry back in the queue and re-throw the error:
          await queue.unshiftRequest(entry);
          throw error;
        }
      }
      console.log("Replay complete!");
    }

yep this is what worked for me. the destructure + the use of queue.shiftRequest() rather than this.shiftRequest()

@SebastianGarces
Copy link

Is this something that will be revisited at some point? Having the capability of extending the default onSync behavior would be much better than having to duplicate its behavior from source code to then extend it ourselves.

@CodingDoug
Copy link

I landed here while looking for the Queue observer callbacks suggested in the Queue documentation:

All parts of the storing and replaying process are observable via callbacks.

As one can imagine, I'm disappointed to find out that such callbacks don't actually exist, and have been requested 5 years ago.

Please consider this a +1 for such observability (and also maybe consider updating the documentation in the meantime).

@tomayac
Copy link
Member

tomayac commented Apr 25, 2024

Hi there,

Workbox is moving to a new engineering team within Google. As part of this move, we're declaring a partial bug bankruptcy to allow the new team to start fresh. We realize this isn't optimal, but realistically, this is the only way we see it working. For transparency, here're the criteria we applied:

Thanks, and we hope for your understanding!
The Workbox team

@tomayac tomayac closed this as completed Apr 25, 2024
@siddhartha8916
Copy link

onSync: async ({ queue }) => {
    let entry;
    while ((entry = await queue.shiftRequest())) {
      const body: I_AddUser_Body = await entry.request.json();

      // Update timestamp to current time
      body.timestamp = Date.now(); // Updating timestamp to current time

      // Create a new request with updated body
      const updatedRequest = new Request(entry.request.url, {
        method: entry.request.method,
        headers: entry.request.headers,
        body: JSON.stringify(body),
      });

      try {
        await fetch(updatedRequest);
        console.log("Replay successful for request", updatedRequest);
      } catch (error) {
        console.error("Replay failed for request", updatedRequest, error);

        // Put the entry back in the queue and re-throw the error:
        await queue.unshiftRequest(entry);
        throw error;
      }
    }
    console.log("Replay complete!");
  }

A custom timestamp manager for updating req body before sync event fires

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