Skip to content

API: batch status update in the SMS queue processor is unawaited, failures vanish #279

Description

@vernu

In the SMS queue processor (api/src/gateway/queue/sms-queue.processor.ts, lines 72-83) the batch status update is fired without await, and its error handler rethrows into nowhere:

try {
  this.smsBatchModel
    .findByIdAndUpdate(smsBatchId, {
      $set: { status: 'processing' },
    })
    .exec()
    .catch((error) => {
      this.logger.error(
        `Failed to update sms batch status to processing ${smsBatchId}`,
        error,
      )
      throw error
    })

  const response = await firebaseAdmin.messaging().sendEach(fcmMessages)

Two problems:

  • the promise isn't awaited, so the enclosing try/catch can't see the failure, and the throw error inside .catch just creates an unhandled rejection
  • when the update fails, the batch silently stays pending until the 20 minute sweep in sms-status-update.task.ts rewrites it to unknown, even though the messages went out fine

Fix is adding await (and dropping the rethrow, since the outer catch then handles it). One line, but worth a test that the batch actually transitions to processing.

Metadata

Metadata

Assignees

No one assigned

    Labels

    apiBackend APIbugSomething isn't workinggood first issueGood for newcomers

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions