So I'm writing a node.js script that uses Playwright to send different messages at a set interval in a browser. I'm running into an issue with an asynchronous part of the script.

I initially had it set up in this way

async start() {
  this.interval = setInterval(async () => {
    await sendMessages(this.messages);
  }, 1000);
}

I'm pretty sure the await keyword does nothing here and just executes the next iteration after 1000 ms regardless of whether sendMessages is finished executing or not. This was resulting in messages being double sent when they hit their interval requirement for sending again.

This code works, and is the subject of my question:

async start() {
  while (!this.stopFlag) {
    await sendMessages(this.messages);
    await timeout(1000);
  }
}

timeout is a simple function that resolves a Promise after a setTimeout call. However, this seems kind of hacky and I don't really like this way of implementation. Does anyone have a more clever way to execute something like this?

Here are some more code details:

Messenger.js

import Message from './Message.js'
import { timeout } from './Timeout.js'
export default class Messenger {
  constructor(messages, page) {
    this.page = page;
    this.messages = messages.map((msg) => new Message(msg));
    this.stopFlag = false;
  }

  async start() {
    while (!this.stopFlag) {
      await this.sendMessages();
      await timeout(1000);
    }
  }

  async sendMessages() {
    for (const message of this.messages) {
      if (message.shouldSend()) {
        await message.send(this.page);
      }
    }
  }

  stop() {
    this.stopFlag = true;
  }
}

Message.js

export default class Message {
  constructor(message) {
    this.text = message.text;
    this.timing = message.timing;
    this.lastSent = null;
  }

  async send(page) {
    logMessage(`Sending message "${this.text}" in channel.`);
    await playwrightFunctionToSendMessage();
    this.lastSent = new Date();
  }

  shouldSend() {
    // returns a boolean based on this.timing and this.lastSent 
  }
}

CodePudding user response：

Instead of the stop method reassigning this.stopFlag, have it clear the interval or timeout.

async start() {
    if (this.timeoutId) return; // timeout is already running - don't start another
    const startTimeout = () => {
        this.timeoutId = setTimeout(() => {
            this.sendMessages()
                .then(() => {
                    if (this.timeoutId) startTimeout();
                })
                .catch(handleErrors); // don't forget this part
        }, 1000); // 1 second between finish of last sendMessages and start of next
    };
    startTimeout();
}
stop() {
    clearTimeout(this.timeoutId);
    this.timeoutId = null;
}

CodePudding user response：

await inside setInterval will not work as expected and will keep adding to the calls to the queue and move on.

   this.interval = setInterval(async () => {
    await sendMessages(this.messages); // will not be async as expected
  }, 1000);  

if there is a possibility that your logic could take longer to execute than the interval time, it is recommended that you recursively call a named function using setTimeout(). For example, if using setInterval() to poll a remote server every 5 seconds, network latency, an unresponsive server, and a host of other issues could prevent the request from completing in its allotted time. As such, you may find yourself with queued up XHR requests that won't necessarily return in order.

Challenge is if you need long-running processes to execute at set times and need a result before the next execution time. Obviously, if it takes longer than the interval to process then you will need a more advanced way to keep track of batches and or reduce the workload.

But if you simply want a way to wait until completion before executing the process again after a delay, you could do:

function sendMessages() {
  return new Promise((resolve) => {
    setTimeout(() => {
      console.info('done sending');

      return resolve();
    }, 2000);
  })
}

let stopFlag = false;

async function process() {
  if (stopFlag) {
    console.info('stopping');
  } else {
    // wait till complete and set a new timeout
    await sendMessages();
    setTimeout(() => process(), 1000);
  }
}

process();

// trigger a stop
setTimeout(() => {
  stopFlag = true;
}, 5000);