1

I am making a chrome extension (mv3). Based on user activity, the content.js passes a message to the background.js which then calls an async function to add data in Google Docs using Docs API.

I want each request to execute only after the previous one has finished running. I am using chrome.runtime.sendMessage to send a message from content.js and don't see a way of calling background.js serially from there. So I need a way of executing them one by one in background.js only. The order of these requests is also important (but if the order of the requests gets changed by one/two places, I think that would still be okay from a user perspective).

I tried something and it is working but I am not sure if I am missing some edge cases, because I was unable to find the approach in any other answers -

The approach I used is: I use a stack like structure to store requests, use setInterval to check for any pending requests and execute them serially.

content.js:

chrome.runtime.sendMessage({message});

background.js:

let addToDocInterval = "";
let addToDocCalls = [];

async function addToDoc(msg) {
    // Await calls to doc API
}

async function addToDocHelper() {
    if(addToDocCalls.length === 0)
        return;

    clearInterval(addToDocInterval)

    while(addToDocCalls.length > 0) {
        let msg = addToDocCalls.shift();
        await addToDoc(msg);
    }

    addToDocInterval = setInterval(addToDocHelper, 1000);
}

chrome.runtime.onMessage.addListener((msg) => {
    // Some other logic
    addToDocCalls.push(msg);
})

addToDocInterval = setInterval(addToDocHelper, 1000);

Is this approach correct? Or is there any better way to do this?

Parth Kapadia
  • 507
  • 6
  • 18
  • Your use of `setInterval()` is odd. May as well just use `setTimeout()` since you just call `clearInterval()` the first the interval timer fires anyway and then set a new interval after you're done processing. – jfriend00 Nov 16 '22 at 06:25
  • @jfriend00 But If the array length is 0, the interval isn't cleared, and the function `returns` before it. Till the time we get the first request, it has to keep executing every second. How will that work with timeout? – Parth Kapadia Nov 16 '22 at 06:27
  • Just remove that initial check for the length. You don't need it at all. The `while` loop already checks it. Remove the `clearInterval()` and change the `setInterval()` to a `setTimeout()`. Then, if the length is zero, you just call another `setTimeout()`. – jfriend00 Nov 16 '22 at 06:33
  • 1
    I'm kind of wondering why you're polling the length with a timer. That's not usually the best way to do things in an event driven system. Why not just have the code that adds something to the array trigger an event so you can process the array on that event (if not already processing it). – jfriend00 Nov 16 '22 at 06:35
  • @jfriend00 How to achieve that? Using some variable as a lock? Won't that cause two requests to get executed simultaneously if they are received exactly at the same time? – Parth Kapadia Nov 16 '22 at 06:36
  • This - "have the code that adds something to the array trigger an event so you can process the array on that event (if not already processing it)". If two requests are received at the same time, the code adding to the array will find that it is not already processing any requests (for both new requests) and will start processing them together? – Parth Kapadia Nov 16 '22 at 06:43
  • See my answer below for how to achieve that. – jfriend00 Nov 16 '22 at 06:59

3 Answers3

1

I'd suggest changing several things.

  1. Don't use timers polling the array. Just initiate processing the array anytime you add a new item to the array.
  2. Keep a flag on whether if you're already processing the array so you don't start duplicate processing.
  3. Use a class to encapsulate this functionality into an object.
  4. Encapsulate the addToDocCalls array and adding to it so your class is managing it and outside code just calls a function to add to it which also triggers the processing. Basically, you're making it so callers don't have to know how the insides work. They just call helper.addMsg(msg) and the class instance does all the work.

Here's an implementation:

async function addToDoc(msg) {
    // Await calls to doc API
}

class docHelper {
    constructor() {
        this.addToDocCalls = [];
        this.loopRunning = false;
    }
    addMsg(msg) {
        // add item to the queue and initiate processing of the queue
        this.addToDocCalls.push(msg);
        this.process();
    }
    async process() {
        // don't run this loop twice if we're already running it
        if (this.loopRunning) return;

        try {
    
            this.loopRunning = true;
        
            // process all items in the addToDocCalls we have
            while(this.addToDocCalls.length > 0) {
                let msg = addToDocCalls.shift();
                await addToDoc(msg);
            }
        } finally {
            this.loopRunning = false;  
        }
    }
}

const helper = new docHelper();


chrome.runtime.onMessage.addListener((msg) => {
    // Some other logic
    helper.addMsg(msg);
});

So, process() will run until the array is empty. Any interim calls to addMsg while process() is running will add more items to array and will call process() again, but the loopRunning flag will keep it from starting duplicate processing loops. If addMsg() is called while process is not running, it will start the process loop.

P.S. You also need to figure out what sort of error handling you want if addToDoc(msg) rejects. This code protects the this.loopRunning flag if it rejects, but doesn't actually handle a reject error. In code like this that is processing a queue, often times all you can really do is log the error and move on, but you need to decide what is the proper course of action on a rejection.

jfriend00
  • 683,504
  • 96
  • 985
  • 979
  • My only doubt is: If `process` is not running right now and `loopRunning` is `false`. Two requests came and `helper.addMsg(msg)` was called for both of them at the same time. `if (this.loopRunning) return;` - This condition was evaluated to `false` for both of them, and then two instances of `process` start executing simultaneously.. is this possible? So probably what I want is "mutual exclusion", does this guarantee it? - https://www.scaler.com/topics/mutual-exclusion-in-os/ Correct me if I am wrong... – Parth Kapadia Nov 16 '22 at 07:13
  • 1
    @ParthKapadia - There is no "running at the same time" in nodejs. It runs your Javascript in a single thread so in this code there is no possibility of running two `process()` loops simultaneously. So, one `helper.addMsg()` is guaranteed to finish and return before another one can get called. Or, put another way, you can't get two `chrome.runtime.onMessage.addListener()` events in flight at the same time. One event will get processed and return before the next one triggers regardless of when they actually originated. – jfriend00 Nov 16 '22 at 07:21
  • 1
    @ParthKapadia - Or to your specific worry, it is not possible for `process()` to get called again between the statements `if (this.loopRunning) return;` and `this.loopRunning = true`. Javascript in nodejs is not pre-emptive, multi-threaded. It's non-pre-emptive, single threaded (which drastically, drastically simplifies coding in it) compared to true pre-emptive threading. If it were pre-emptive threaded, this would have to use mutexes to avoid concurrency problems. But, that is not required here. – jfriend00 Nov 16 '22 at 07:26
1

You don't need to use setTimeout. You do not even need a while loop.

let addToDocInterval = "";
let addToDocCalls = [];
let running = false;

async function addToDoc(msg) {
    // Await calls to doc API
}

async function addToDocHelper() {
    if(running || addToDocCalls.length === 0)
        return;

    running = true;

    let msg = addToDocCalls.shift();
    await addToDoc(msg);

    running = false;

    addToDocHelper();
}

chrome.runtime.onMessage.addListener((msg) => {
    // Some other logic
    addToDocCalls.push(msg);
    addToDocHelper();
});

The code should be self explanatory. There is no magic.

Molda
  • 5,619
  • 2
  • 23
  • 39
  • 1
    So, you're using recursion instead of a while loop? Why? Also, what do you think happens if `await addToDoc()` rejects? Your `running` variable gets stuck on `true` forever and nothing else ever gets processed. – jfriend00 Nov 16 '22 at 07:04
  • @jfriend00 I agree that await should be in a try/catch block but that is not part of the question so i think it's ok to leave it out. Once you start using it you should know how to handle possible errors. And loop vs recursion? Recursion makes the code cleaner to me. Based on the expected usage i assume it is not going to be too cpu intensive. – Molda Nov 16 '22 at 13:45
0

Here is a generic way to run async tasks sequentially (and add more tasks to the queue at any time).

const tasks = [];
let taskInProgress = false;
async function qTask(newTask) {
  if (newTask) tasks.push(newTask);
  if (tasks.length === 0) return;
  if (taskInProgress) return;

  const nextTask = tasks.shift();
  taskInProgress = true;
  try {
    await nextTask();
  } finally {
    taskInProgress = false;
      
    //use setTimeout so call stack can't overflow
    setTimeout(qTask, 0);
  }
}



//the code below is just used to demonstrate the code above works

async function test() {
  console.log(`queuing first task`);
  qTask(async () => {
    await delay(500); //pretend this task takes 0.5 seconds
    console.log('first task started');
    throw 'demonstrate error does not ruin task queue';
    console.log('first task finished');
  });

  for (let i = 0; i < 5; i++) {
    console.log(`queuing task ${i}`)
    qTask(async () => {
      await delay(200); //pretend this task takes 0.2 seconds
      console.log(`task ${i} ran`);
    });  
  }

  await delay(1000); //wait 1 second

  console.log(`queuing extra task`);
  qTask(async () => {
    console.log('extra task ran');
  }); 

  await delay(3000); //wait 3 seconds

  console.log(`queuing last task`);
  qTask(async () => {
    console.log('last task ran');
  }); 
}
test();

function delay(ms) {
  return new Promise(resolve => {
    setTimeout(resolve, ms);
  });
}
Rocky Sims
  • 3,523
  • 1
  • 14
  • 19