[webhooks] use scheduling instead of polling

[capcom6]

Summary by CodeRabbit

• New Features
  • Webhook retries now run based on due retry times, using the next available attempt time to automatically reschedule processing.
  • Job startup now supports an optional initial delay for improved timing control.
• Bug Fixes
  • Rescheduling now replaces any existing queued run to prevent stale or duplicate execution timing.
  • Batch processing is more resilient by isolating cancellation and failure handling per webhook item to reduce cross-impact.

Greptile Summary

This PR replaces continuous polling with a scheduling approach for webhook retries: after draining all currently-due webhooks, the worker reads the minimum next_attempt time from the DB and re-enqueues itself with that exact delay using APPEND_OR_REPLACE, so the processor only wakes when work is genuinely ready. It also fixes two previously-flagged issues — the HttpClient leak (client is now lazy and closed in finally) and CancellationException being swallowed by the generic catch (e: Exception) block.

• Scheduling logic (doWork): loop exits when hasDueWebhooks() is false, then getNextAttemptTime() drives a targeted self-reschedule; APPEND_OR_REPLACE is used so an urgent immediate REPLACE from SendWebhookWorker correctly wins.
• Query changes (WebhookQueueDao): scheduledWebhooksCount replaced by time-bounded dueWebhooksCount(currentTime) and new getNextAttemptTime() (MIN of pending/failed next_attempt).
• Resilience (processBatch): each webhook is now wrapped in its own withContext(NonCancellable) + try/catch, so a per-item failure cannot abort the rest of the batch.

Confidence Score: 4/5

• Safe to merge; the scheduling logic, policy interactions, and cancellation handling are all correct. Two minor style nits remain (lazy client close guard and negative delay clamping) but neither affects correctness.
• The core scheduling design is sound: hasDueWebhooks() and getNextAttemptTime() use consistent time-bounded queries, APPEND_OR_REPLACE correctly yields to urgent REPLACE enqueues from SendWebhookWorker, and the CancellationException rethrow and HttpClient lazy/close fixes address the two previously-reported defects. The remaining comments are purely style suggestions (clamping a negative delay before logging and guarding the lazy client close in finally).
• WebhookQueueProcessorWorker.kt — the finally { client.close() } block and the raw delayMs computation are the two spots worth a quick look before merging.

Important Files Changed

Filename	Overview
app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt	Replaces scheduledWebhooksCount with time-bounded dueWebhooksCount(currentTime) and adds getNextAttemptTime(); both queries are correct and the KDoc accurately describes the new semantics.
app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt	Replaces hasScheduledWebhooks with hasDueWebhooks (passes current time) and adds getNextAttemptTime thin wrapper; logic is straightforward and correct.
app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt	Core logic switch from polling to scheduling; fixes CancellationException swallowing and the HttpClient lazy/close issues; adds per-webhook NonCancellable isolation. One P2 style issue: client.close() in finally always forces lazy initialisation even on early failures.

Sequence Diagram

sequenceDiagram
    participant SW as SendWebhookWorker
    participant WM as WorkManager
    participant P as WebhookQueueProcessorWorker
    participant R as WebhookQueueRepository

    SW->>R: enqueueWebhook(url, payload)
    SW->>WM: "start(policy=REPLACE, delay=0)"
    WM->>P: doWork()

    loop Until no due webhooks
        P->>R: hasDueWebhooks()
        R-->>P: true
        P->>R: getPendingWebhooks()
        loop Per webhook (NonCancellable)
            P->>R: startProcessing(id)
            P->>P: sendWebhook()
            alt Success
                P->>R: completeWebhook(id)
            else Failure
                P->>R: scheduleRetry(id, backoff)
            end
        end
    end

    P->>R: cleanupOldEntries()
    P->>R: getNextAttemptTime()
    R-->>P: nextAttempt (future time)
    P->>WM: "start(policy=APPEND_OR_REPLACE, delay=nextAttempt-now)"
    P-->>WM: Result.success()

    Note over WM,P: Delayed worker wakes at next retry time
    WM->>P: doWork() (after delay)

Loading

_{Reviews (14): Last reviewed commit: "[webhooks] use scheduling instead of pol..." | Re-trigger Greptile}

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: a4f0480b-1b62-41c5-a4c2-470e73dffffd

📥 Commits

Reviewing files that changed from the base of the PR and between 7f8418e and 8b9ad1b.

📒 Files selected for processing (3)

• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt

🚧 Files skipped from review as they are similar to previous changes (3)

• app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt

---

Walkthrough

Adds DAO queries for due webhook counts and next attempt time, updates the repository to use due-webhook checks, and changes the worker to accept initial delays, reschedule from the next attempt time, and process each webhook inside a NonCancellable block.

Changes

Webhook Due-Time Scheduling

Layer / File(s)	Summary
DAO due-time queries app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt	Adds dueWebhooksCount(currentTime) and getNextAttemptTime() query methods.
Repository due-webhook API app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt	Replaces hasScheduledWebhooks() with hasDueWebhooks() and getNextAttemptTime(), plus a formatting-only change in enqueueWebhook.
Worker start delay wiring app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt	start() gains initialDelayMs, applies it to the work request, and switches unique work policy to REPLACE.
Worker loop and per-webhook cancellation boundary app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt	doWork() uses due-webhook checks and self-reschedules from the next attempt time; processBatch() wraps each webhook in withContext(NonCancellable).
Estimated code review effort: 4 (Complex)	~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant WorkManager
  participant WebhookQueueProcessorWorker
  participant WebhookQueueRepository
  participant WebhookQueueDao

  WorkManager->>WebhookQueueProcessorWorker: start(initialDelayMs)
  WebhookQueueProcessorWorker->>WebhookQueueRepository: hasDueWebhooks()
  WebhookQueueRepository->>WebhookQueueDao: dueWebhooksCount(currentTime)
  WebhookQueueProcessorWorker->>WebhookQueueRepository: getNextAttemptTime()
  WebhookQueueRepository->>WebhookQueueDao: getNextAttemptTime()
  WebhookQueueProcessorWorker->>WorkManager: enqueue next run with setInitialDelay(...)

Loading

Possibly related PRs

• capcom6/android-sms-gateway#337: Updates overlapping webhook worker processing and failure-handling callsites.

🚥 Pre-merge checks | ✅ 5

✅ Passed checks (5 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title accurately summarizes the main change: webhooks now use scheduling instead of continuous polling.
Docstring Coverage	✅ Passed	No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.
Linked Issues check	✅ Passed	Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check	✅ Passed	Check skipped because no linked issues were found for this pull request.

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch webhooks/schedule-worker-instead-of-loop

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[coderabbitai]

Actionable comments posted: 2

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt`:
- Around line 106-108: The WebhookQueueProcessorWorker start path is replacing
the currently running unique work, which can cancel an active processor. Update
the enqueueUniqueWork call in WebhookQueueProcessorWorker.start() so it does not
use REPLACE for the live worker; instead use a non-replacing policy or split
rescheduling of delayed retries into a separate path while preserving the
existing WORK_NAME identity.
- Around line 161-186: The scheduling branch in
WebhookQueueProcessorWorker.after the cleanup/getNextAttemptTime flow drops
short-delay retries when nextAttempt is within MAX_FOREGROUND_DELAY_MS, leaving
pending webhooks unscheduled. Update the logic in WebhookQueueProcessorWorker
(around the cleanupOldEntries/getNextAttemptTime/start path) so short future
retries are either delayed in the worker or rescheduled immediately instead of
falling through to Result.success(); keep the existing start(...) call for long
delays and add the missing branch for near-term attempts.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 0acd7d05-9677-400a-a263-43508f558922

📥 Commits

Reviewing files that changed from the base of the PR and between abe8f76 and b7759f1.

📒 Files selected for processing (3)

• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt

[coderabbitai]

Actionable comments posted: 1

🤖 Prompt for all review comments with AI agents

Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt`:
- Around line 262-291: The webhook processing block in
WebhookQueueProcessorWorker.processBatch keeps the entire send path inside
withContext(NonCancellable), which prevents cancellation during
sendWebhook(webhook). Move NonCancellable so it wraps only the cleanup/update
steps after sendWebhook returns, specifically
webhookRepository.completeWebhook(webhook.id) and handleWebhookFailure(...),
while leaving sendWebhook(webhook) cancellable inside the main try flow.

🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

• Push a commit to this branch (recommended)
• Create a new PR with the fixes

---

ℹ️ Review info

⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: ab0c52d4-ec46-48c8-9a0e-9d7cbec65780

📥 Commits

Reviewing files that changed from the base of the PR and between b7759f1 and cacbc7a.

📒 Files selected for processing (3)

• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/workers/WebhookQueueProcessorWorker.kt

🚧 Files skipped from review as they are similar to previous changes (2)

• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueDao.kt
• app/src/main/java/me/capcom/smsgateway/modules/webhooks/db/WebhookQueueRepository.kt

[github-actions]

🤖 Pull request artifacts

file	commit
app-release.apk	855361d
app-release.aab	855361d
app-insecure.apk	855361d
app-insecure.aab	855361d

[github-actions]

This PR is stale because it has been open for 7 days with no activity.

[capcom6]

@greptile review