fix: limit retries when calling ensureBucket

[DuncanBetts]

When wrapping a call to client.ensureBucket with p-retry, passing forever: true may have caused issues such as #176 and #60 , for users whose ActivityWatch server is not reachable, since every new heartbeat() calls a new ensureBucket(), resulting in 60 concurrent calls an hour after the user opens their browser, and considerably more after a few days.

As a side-effect, removes deprecated p-retry forever.

If I understand correctly, every 60s the onAlarm listener; heartbeatAlarmListener is called, which in turn awaits the heartbeat function, which in turn calls sendHeartbeat, which retries 3x and onFailedAttempt calls ensureBucket.

What I haven't thought through fully is whether this means a browser notification to inform the user "Unable to send event to server", "Please ensure that ActivityWatch is running" is never sent, as perhaps in that scenario, ensureHeartbeat simply retries forever instead?

Replacing forever: true with three retries before giving up, as every 60s heartbeatAlarmListener will be called regardless, if I understand correctly?

[greptile-apps]

Greptile Summary

This PR addresses a resource-accumulation bug where ensureBucket was wrapped with p-retry's forever: true, causing each call to retry indefinitely. Because sendHeartbeat (and therefore ensureBucket) is triggered every 60 seconds via the alarm listener, after prolonged browser uptime dozens of concurrent, never-terminating retry loops would accumulate, which is the likely root cause of issues like #176 and #60. The fix is correct and well-scoped: replacing forever: true with retries: 3 makes the retry behavior bounded and consistent with how sendHeartbeat and detectHostname already behave in the same file.

• One latent bug is now exposed: The onFailedAttempt callback in sendHeartbeat calls ensureBucket(...).then(() => {}) with no .catch(). When forever: true was in effect, ensureBucket would never reject, so this was harmless. With retries: 3, ensureBucket can reject after four failed attempts, producing an unhandled promise rejection.
• Notification behaviour improves: As the PR description wonders about — with the old forever: true, the sendHeartbeat retry chain could never terminate, so the catch block emitting the "Unable to send event to server" notification would rarely (if ever) fire. With retries: 3, that path works as intended.
• Retry multiplication is bounded: sendHeartbeat runs up to 4 attempts, each of which triggers ensureBucket (up to 4 attempts) via onFailedAttempt, for a worst-case of 16 client.ensureBucket calls — still bounded and acceptable."

Confidence Score: 4/5

Safe to merge — the fix correctly bounds retry behaviour and eliminates the accumulation bug.

The core change is a clear improvement over the previous unbounded retry. One pre-existing latent bug (unhandled promise rejection in onFailedAttempt) becomes reachable with this change and should be addressed, but it does not block the primary fix.

src/background/client.ts line 77 — the ensureBucket call inside onFailedAttempt needs a .catch() handler now that it can actually reject.

Important Files Changed

Filename	Overview
src/background/client.ts	Replaces forever: true with retries: 3 in ensureBucket; exposes a latent unhandled promise rejection in sendHeartbeat's onFailedAttempt callback.

Sequence Diagram

sequenceDiagram
    participant Alarm as heartbeatAlarmListener
    participant HB as heartbeat()
    participant SH as sendHeartbeat() [retries:3]
    participant EB as ensureBucket() [retries:3]
    participant AW as AWClient

    Alarm->>HB: every 60s
    HB->>SH: sendHeartbeat(...)
    SH->>AW: client.heartbeat()
    AW-->>SH: ❌ fail
    SH->>EB: onFailedAttempt → ensureBucket()
    EB->>AW: client.ensureBucket() [up to 4 tries]
    AW-->>EB: ❌ fail (all 4)
    EB-->>SH: reject (⚠️ unhandled — no .catch())
    SH->>AW: retry client.heartbeat() [up to 3 more]
    AW-->>SH: ❌ fail all retries
    SH-->>HB: catch → emitNotification("Unable to send event")

Loading

Comments Outside Diff (1)

1. src/background/client.ts, line 77 (link)

   Newly reachable unhandled promise rejection

   With forever: true, ensureBucket would retry indefinitely and never reject, so the missing .catch() on this call was never reachable. With retries: 3, ensureBucket can now exhaust all of its attempts and reject — but the resulting rejected promise has no error handler. This creates an unhandled promise rejection every time the bucket creation fully fails, which in browser extension contexts can produce console warnings or be caught by global unhandled-rejection handlers in unexpected ways.

   Consider adding a .catch() to swallow or log the error gracefully:

_{Reviews (1): Last reviewed commit: "fix: limit retries when calling client.e..." | Re-trigger Greptile}

[DuncanBetts]

• One latent bug is now exposed: The onFailedAttempt callback in sendHeartbeat calls ensureBucket(...).then(() => {}) with no .catch(). When forever: true was in effect, ensureBucket would never reject, so this was harmless. With retries: 3, ensureBucket can reject after four failed attempts, producing an unhandled promise rejection.

I've just added ed1cf63 to ensure the ServiceWorker doesn't crash if ensureBucket retries do fail.