fix(digest): a switch read error mid-run is a blip, not a flip

backend/__tests__/unit/services/digestEmailService.test.js

@@ -228,6 +228,34 @@ describe('digestEmailService', () => {
     expect(mockSendEmail).toHaveBeenCalledTimes(1);
   });
 
+  // sprint-review on #2277: the per-recipient read sits BEFORE the send, so a
+  // flip between the run's first read and the first send stops that email too.
+  it('a flip before the first send stops even the first email', async () => {
+    const user = await createUser();
+    const digest = await createDigest(user);
+    const read = jest.spyOn(SystemSetting, 'findOne')
+      .mockImplementationOnce(() => ({ lean: async () => null }))
+      .mockImplementationOnce(() => ({ lean: async () => ({ value: { enabled: false } }) }));
+
+    await expect(digestEmailService.sendDigestEmails([runEntry(digest)]))
+      .resolves.toEqual(expect.objectContaining({ sent: 0, disabled: true }));
+    expect(mockSendEmail).not.toHaveBeenCalled();
+    read.mockRestore();
+  });
+
+  // ...and an unreadable switch mid-run is a read blip, not a flip: the switch
+  // was on at the start, and a summary skipped here is never retried.
+  it('a read error mid-run does not stop the rest of the run', async () => {
+    const digests = [await createDigest(await createUser()), await createDigest(await createUser())];
+    const read = jest.spyOn(SystemSetting, 'findOne')
+      .mockImplementationOnce(() => ({ lean: async () => null }))
+      .mockImplementation(() => { throw new Error('mongo blip'); });
+
+    await expect(digestEmailService.sendDigestEmails(digests.map(runEntry)))
+      .resolves.toEqual(expect.objectContaining({ sent: 2, disabled: false }));
+    read.mockRestore();
+  });
+
   it('sends nothing when the switch cannot be read', async () => {
     const user = await createUser();
     const digest = await createDigest(user);

backend/services/digestEmailService.ts

@@ -18,15 +18,20 @@ const SystemSetting = require('../models/SystemSetting');
  */
 export const DIGEST_SENDING_KEY = 'digest.sending';
 
-// Unreadable means do not send: for email to people, the switch fails closed.
-const digestSendingEnabled = async (): Promise<boolean> => {
+// 'unknown' is a read that failed, which is not the same as a flip: at the
+// start of a run it means do not send (for email to people the switch fails
+// closed), but mid-run the switch is already known to have been on, so only a
+// definite 'off' stops the run. A summary skipped by a stop is never retried
+// (each run sends only its own), so a read blip must not forfeit the rest.
+type DigestSendingState = 'on' | 'off' | 'unknown';
+const digestSendingState = async (): Promise<DigestSendingState> => {
   try {
     const row = await SystemSetting.findOne({ key: DIGEST_SENDING_KEY })
       .lean() as { value?: { enabled?: unknown } } | null;
-    return row?.value?.enabled !== false;
+    return row?.value?.enabled === false ? 'off' : 'on';
   } catch (error) {
-    console.warn('Digest email delivery skipped: the sending switch could not be read:', (error as Error).message);
-    return false;
+    console.warn('Digest email: the sending switch could not be read:', (error as Error).message);
+    return 'unknown';
   }
 };
 
@@ -132,8 +137,11 @@ export class DigestEmailService {
       disabled: false,
     };
 
-    if (!await digestSendingEnabled()) {
-      console.warn(`Digest email delivery skipped: switched off (${DIGEST_SENDING_KEY}).`);
+    const atStart = await digestSendingState();
+    if (atStart !== 'on') {
+      console.warn(atStart === 'off'
+        ? `Digest email delivery skipped: switched off (${DIGEST_SENDING_KEY}).`
+        : `Digest email delivery skipped: the sending switch is unreadable (${DIGEST_SENDING_KEY}).`);
       result.disabled = true;
       return result;
     }
@@ -161,7 +169,7 @@ export class DigestEmailService {
       // Read again per recipient (sprint-review on #2275): the switch is for an
       // incident, and an incident is often a run that is already sending.
       // eslint-disable-next-line no-await-in-loop
-      if (!await digestSendingEnabled()) {
+      if (await digestSendingState() === 'off') {
         console.warn(`Digest email delivery stopped mid-run: switched off (${DIGEST_SENDING_KEY}).`);
         result.disabled = true;
         break;

docs/runbooks/daily-digest.md

@@ -30,7 +30,10 @@ m.connect(process.env.MONGO_URI).then(async () => {
 ```
 
 To turn it back on, run the same command with `enabled: true`, or delete the
-row. If the row can't be read, nothing is sent: the switch fails closed. A run
+row. If the row can't be read when a run starts, nothing is sent: the switch
+fails closed. A read error partway through a run that started switched on is
+treated as a blip, not a flip, so it doesn't stop the run; only a definite
+`enabled: false` does. A run
 logs `Digest email delivery skipped: switched off (digest.sending).` when it
 starts switched off, and `Digest email delivery stopped mid-run: ...` when the
 switch is flipped while it's sending.