fix: SendGrid webhook batching, idempotency-key stability, TTS circuit breaker, and cross-tenant job isolation - #1249
Merged
Merged
Conversation
handle_sendgrid_webhook looped over events sequentially, awaiting each process_event one at a time. A single batch of hundreds of events could hold a DB pool connection through hundreds of sequential round trips before responding, risking SendGrid webhook timeouts and pool contention. Process events concurrently with bounded parallelism (buffer_unordered) instead.
…lock hour bucket idempotency_key rotated its hour bucket off SystemTime::now() on every call. mark_failed's exponential backoff (2/4/8/16/32/~64min...) crosses that 1-hour boundary by the 6th retry, so a retry scheduled more than an hour after the original attempt computed a different key and would not be deduplicated -- risking a duplicate send if the original attempt had actually succeeded at SendGrid before the worker crashed. The hour bucket is now derived from the job's original creation time (job.created_at) instead of the current time, so the key stays stable across a job's full retry lifetime regardless of backoff delay.
…etry attempt withRetry() wrapped _callProvider(), which itself called breaker.fire() on every retry attempt. A single failed user request could generate up to maxRetries+1 breaker failures instead of one, tripping the breaker's volumeThreshold/errorThresholdPercentage far earlier than intended. Once open, the breaker's OpenCircuitError was rewrapped as a 503 TTSProviderError, which isRetryable() treats as retryable, so the outer retry loop kept retrying an open breaker with full exponential backoff instead of failing fast -- the opposite of the breaker's purpose. Move the retry/backoff loop inside each breaker's wrapped action so opossum's fire() is called exactly once per external request. When the circuit is open, fire() now rejects immediately without invoking the action at all, so callers fail fast with zero retries.
jobStore was a single global Map with no owner field. GET /tts/jobs and GET /tts/job/:id returned every job in the process -- including other tenants' narration text, voice, and file paths -- to any caller that passed the (optional) auth check, since neither endpoint filtered by the requesting credential. Tag each job with the owner identity resolved from its creating credential (the API key itself, or a JWT's `sub` claim; "anonymous" when no auth is configured), and require getJob/listJobs to match it. getJob returns the same 404 for "not found" and "belongs to another tenant" so a credential can't distinguish the two by probing IDs. Internal operational code (health checks, queue-depth metrics) that needs a cross-tenant view now uses the new listAllJobsUnscoped(). Also fixes /tts/enqueue and /tts/generate, which were never passing the authenticated request's credential down to the service layer at all.
… retry+timeout logic
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Conflict-resolved re-submission of #1246 (fork PR, no push access to update it directly). Merges main in and reconciles the circuit-breaker retry/timeout logic with #1246's original changes. Closes #1246 in favor of this.