fix(speakers): remove the quadratic hydration and the unordered pagination behind the bulk send ( promo codes ) - #593
Merged
smarcet merged 3 commits intoSep 3, 2026
Conversation
…a speaker as sent SpeakersPromoCodeTrait::setEmailSent found the AssignedPromoCodeSpeaker to mark through $this->owners->filter(). Collection::filter() initializes the whole collection regardless of the EXTRA_LAZY mapping, and the closure then dereferenced ->getSpeaker()->getEmail() on every element, lazily hydrating one PresentationSpeaker per element as well. Every speaker of a bulk send shares the same promo code, so the cost of marking speaker N grew with the number of speakers already assigned to it, and every speaker of the run stayed pinned in the identity map. Over a 683 recipient send that is quadratic hydration on top of a heap that only grows. The lookup is now a targeted query. The e-mail matching mirrors PresentationSpeaker::getEmail(), which is computed rather than mapped: the member's e-mail wins, and the registration request's is only used when the speaker has no member. Behaviour is unchanged. The tests were written against the previous implementation and pass unmodified against this one. They cover both branches of the e-mail precedence, a speaker carrying both a member and a registration request, the scoping to this promo code when the same speaker is assigned to another one, and an unknown recipient. Signed-off-by: smarcet <smarcet@gmail.com>
…n getParametrizedAllIdsByPage The default-order callback of getParametrizedAllIdsByPage was chained to $filter instead of $order, so it only ran when no filter was given. Any filtered page was therefore emitted with LIMIT/OFFSET and no ORDER BY at all, and MySQL makes no promise about the order of such a result: paging through one can skip rows or return the same row on two different pages. The sibling getParametrizedAllByPage already chains the same callback to $order, which is the intended contract - an explicit order wins, otherwise the caller's default applies. This aligns the two. The only caller is DoctrineSpeakerRepository::getSpeakersIdsBySummit, whose callback is the default speaker order (e.id ASC). It never passes an explicit order, and ParametrizedSendEmails substitutes an empty Filter when none was given, so in practice the bulk speaker send has been paging an unordered query. The regression test asserts the generated DQL rather than paged data on purpose: an unordered query frequently happens to come back in a stable order, so a data-level test would pass by luck against the defect. It covers a filtered query, an unfiltered one, and that an explicit order still suppresses the default. Signed-off-by: smarcet <smarcet@gmail.com>
|
Note Currently processing new changes in this PR. This may take a few minutes, please wait... ⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (10)
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Team Run ID: 📒 Files selected for processing (4)
Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review. 📝 WalkthroughWalkthroughThe changes update speaker promo code assignment matching and restore default ordering for paginated ID queries. New tests cover email precedence, case-insensitive matching, promo code isolation, unknown recipients, and explicit or default query ordering. ChangesSpeaker promo code email matching
Paginated ID query ordering
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to The PR replaces inefficient speaker sent-state lookup and restores deterministic ordering for filtered pagination without changing intended behavior; targeted and repository tests are green, so no actionable merge-blocking risk remains. 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
Full details: Title checkExplanation The title accurately identifies both main changes: targeted handling that removes quadratic hydration and corrected pagination ordering for bulk promo-code sends. It is specific and related to the changeset, although the spacing inside the parentheses is unnecessary.
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
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. Comment |
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-593/ This page is automatically updated on each push to this PR. |
Contributor
There was a problem hiding this comment.
🟢 Approval recommended
The fixes are well-scoped, align with existing repository contracts, and are backed by targeted regression tests; the only noted concern is a minor warning-path behavior discrepancy.
Pull request overview
This PR addresses two reliability/performance issues impacting bulk speaker email sends: (1) eliminating quadratic Doctrine hydration when marking a speaker promo code assignment as “sent”, and (2) ensuring paginated ID queries remain deterministic by applying the default ORDER BY even when filters are present.
Changes:
- Refactors speaker promo-code “mark as sent” resolution to use a targeted query instead of filtering an EXTRA_LAZY collection.
- Fixes
DoctrineRepository::getParametrizedAllIdsByPageso the default-order callback is applied whenever no explicit order is provided (even with filters). - Adds focused regression tests covering email-precedence matching/scoping and default-order behavior in filtered pagination.
File summaries
| File | Description |
|---|---|
| tests/SpeakersPromoCodeMarkSentTest.php | Adds coverage proving the new lookup matches PresentationSpeaker::getEmail() precedence and promo-code scoping. |
| tests/ParametrizedAllIdsByPageOrderTest.php | Adds regression tests ensuring default ORDER BY is present for filtered/unfiltered ID pagination and suppressed by explicit order. |
| app/Repositories/DoctrineRepository.php | Applies default order fallback for ID pagination when no explicit order is provided, regardless of filter presence. |
| app/Models/Foundation/Summit/Registration/PromoCodes/Traits/SpeakersPromoCodeTrait.php | Replaces collection filtering with a targeted DQL lookup to avoid quadratic hydration during bulk sends. |
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
Comment on lines
+274
to
+277
| $recipient = strtolower(trim($recipient ?? '')); | ||
|
|
||
| if (empty($recipient)) | ||
| throw new ValidationException("Can't mark the promo_code as sent without a recipient."); |
smarcet
changed the title
…er filter whitelists (#595) * feat(speakers): chunk the bulk speaker email send and unify its filter whitelist SpeakerService::triggerSendEmails dispatched a single ProcessSpeakersEmailRequestJob for the whole matched set, which the ParametrizedSendEmails trait then paged through internally inside that one queued job. A killed or failed job lost the entire run; on 2026-08-31 that lost 183 of 683 speakers mid-send with no trace. triggerSendEmails now resolves the full set of matched speaker ids synchronously (from an explicit speaker_ids payload if given, otherwise by paging getSpeakersIdsBySummit), applies excluded_speaker_ids, de-duplicates, and dispatches one ProcessSpeakersEmailRequestJob per 100-id chunk (SpeakerService::CHUNK_SIZE) - the same job class, unchanged in its own per-speaker processing. A killed or failed job now loses at most one chunk. Every dispatched chunk receives the exact same raw, unparsed $filter value triggerSendEmails itself received - not a Filter object, not the filter used to select ids. That raw value also scopes which of a speaker's presentations count as accepted/alternate/rejected inside SpeakerActionsEmailStrategy::process, which decides the email type sent to that speaker; passing anything else would silently change that. This is asserted directly in the tests via ReflectionObject on the dispatched job's private filter property. Two behavior changes, both deliberate: a filter matching zero speakers now dispatches nothing (previously a single job still ran and could send a "0 sent" outcome e-mail); and duplicate ids are de-duplicated before dispatch (previously a repeated id was emailed twice). While rewriting every filter-parsing call site the send path depends on, unify them onto a new ISpeakerFilterFields interface (OPERATORS + VALIDATION_RULES constants, following the IEmailExcerptService interface-constants precedent already used in this codebase). Three of the four call sites already used the same 19-21 fields; one (SpeakerService's original_filter parse) was missing presentations_track_group_id. The bulk-send endpoint gains member_id/member_user_external_id as valid filter fields as a result - they were already supported by the repository and by the sibling listing/CSV/count endpoints, just never wired into the send path. tests/SpeakerServiceBulkSendChunkingTest.php: 9 tests. Most use an explicit speaker_ids payload with fabricated ids rather than seeded speakers, since Queue::fake() intercepts dispatch before the job's handle() ever runs and that path never queries the repository. Only the filter-based-selection and member_id cases seed real speakers. Coverage: chunk count and non-overlapping slices above/at/below CHUNK_SIZE, zero-match, exclusion, de-duplication, payload key pass-through, raw filter identity across both the explicit-ids and filter-based paths, and that member_id actually narrows the query rather than merely being accepted. Two mutations were run against the implementation to confirm the tests have teeth: swapping the raw filter for the internally-parsed one at dispatch time, and removing the de-duplication step. Both were caught. Signed-off-by: smarcet <smarcet@gmail.com> * refactor(speakers): unify getSpeakers/getSpeakersActivitiesCount/getSpeakersCSV/getAll onto ISpeakerFilterFields getSpeakers(), getSpeakersActivitiesCount(), and getSpeakersCSV() already carried the exact 21-field whitelist ISpeakerFilterFields formalizes (byte-identical across all three). Point them at the shared interface instead of three independently-maintained inline copies. getAll() (the global, non-summit-scoped speaker listing) was planned to widen to the same 21 fields, matching the other three - but that's unsafe. Reproduced directly against getAllByPage(): applying presentations_track_id throws Doctrine\ORM\Query\QueryException ("too few parameters"). Every presentations_*/ has_*_presentations mapping in DoctrineSpeakerRepository::getFilterMappings() hard-codes a :summit bound parameter in its DQL (shared verbatim with the summit-scoped query methods, which bind it on their own base query); getAllByPage()/getAllIdsByPage() never do, because there is no single summit to scope a global listing by. 13 of the 21 fields hit this; only the 8 with no :summit reference (id, not_id, first_name, last_name, email, full_name, member_id, member_user_external_id) are safe on a summit-independent query - exactly getAll()'s original set. getAll() now references a new ISpeakerFilterFields::GLOBAL_OPERATORS/ GLOBAL_VALIDATION_RULES pair covering exactly those 8 fields, documented with the full field-by-field breakdown of why the other 13 don't apply. All four methods end up on one shared interface with zero drift risk; getAll() gains no new fields. Making the :summit clause conditional so those mappings work for both summit-scoped and global callers would be real repository-level work across ~13 shared DQL templates - out of scope here. Also corrects send()'s and getAll()'s Swagger filter descriptions, which were already stale before this change. tests/oauth2/OAuth2SummitSpeakersApiTest.php: testGetAllSpeakersFilteredByMemberId proves the swap to GLOBAL_OPERATORS doesn't regress getAll()'s existing member_id support; testGetAllSpeakersRejectsPresentationScopedFilter proves a presentation-scoped field still returns a clean validation error, not a 500. The other four target methods are covered by the existing functional suite (member/external-id/selection-plan/ media-upload/accepted/rejected/name filters), which passes unchanged - confirming the inline-array-to-constant swap is behavior-preserving there. Signed-off-by: smarcet <smarcet@gmail.com> * fix(speakers): dispatch bulk-send chunks through JobDispatcher::withDbFallback Plain ProcessSpeakersEmailRequestJob::dispatch() left the chunk loop exposed to a queue-backend failure part-way through: some chunks already queued, the rest lost with the aborted request, and an operator retry re-emailing the chunks that already went out (should_resend defaults to true and the admin UI never sends it, so re-runs are not deduplicated today). JobDispatcher::withDbFallback tries the primary connection, fails over to the database queue, and runs the chunk synchronously on a double failure - same pattern as PresentationSubmissionReopenService::notify's per-recipient loop. Each iteration additionally wraps the dispatch in its own try/catch so one chunk whose three fallback tiers all failed cannot abort the sibling chunks that would have succeeded - the chunk-isolation property this whole feature exists for. That catch logs at error level: by then the primary, the database fallback, and the synchronous run have all failed, which is an alert-worthy infrastructure event, not a routine warning. Signed-off-by: smarcet <smarcet@gmail.com> * test(speakers): cover chunk-failure isolation and the send() member filter at the HTTP layer Addresses the three findings from the changes review: - testOneChunkFailingAllFallbackTiersDoesNotAbortSiblingChunks: forces every Bus dispatch (queued and sync) to throw so all three JobDispatcher fallback tiers fail for every chunk, then asserts the loop still visited every chunk (one-plus Log::error per chunk) instead of aborting on the first. Mutation-verified: moving the per-chunk try/catch outside the foreach fails the count; removing it fails on the propagated exception. - testSendSpeakersBulkEmailFilteredByMemberUserExternalId: drives the real PUT send() action with a member_user_external_id filter, exercising the controller's FilterParser::parse + Filter::validate against the shared ISpeakerFilterFields constants and the service's id resolution end to end - the review noted the member filter was only proven at the service layer, and only for member_id. - Rewords the getAll() code comment so it no longer contains the literal "ISpeakerFilterFields::" substring, making Task 2's documented DoD grep count (8) match what the command actually returns. Signed-off-by: smarcet <smarcet@gmail.com> * fix(speakers): report a lost bulk-send chunk from ProcessSpeakersEmailRequestJob::failed() With tries = 1, a chunk whose worker is killed mid-run sits reserved until the connection's retry_after elapses, is re-served, and is marked failed without re-running. Nothing reported that loss: the outcome excerpt is only sent when sendEmails() runs to completion, so a dead chunk left no trace beyond a queue_failed_jobs row - the same silence as the 2026-08-31 incident, capped at 100 speakers instead of the whole run. The new failed() hook (invoked by Job::fail() -> CallQueuedHandler::failed(), including the sync tier of JobDispatcher::withDbFallback) logs the summit, flow event, exception, unprocessed speaker ids and raw filter at error level, and when the payload carries outcome_email_recipient dispatches PresentationSpeakerSelectionProcessExcerptEmail with an ERROR line naming the ids and the cause, so the operator can re-send that chunk by id. The excerpt dispatch is best-effort inside a try/catch so it never masks the original failure; without a recipient the hook only logs and never touches the database. tests/ProcessSpeakersEmailRequestJobFailedHookTest.php invokes the hook directly (Queue::fake stops at dispatch, so the framework plumbing cannot be driven end to end here) and pins both paths. Mutation-verified: dropping the recipient guard fails the "exactly one error line" expectation, dropping the ids from the ERROR line fails the id assertion. Named apart from ProcessSpeakersEmailRequestJobTest.php, which exists on another branch. Signed-off-by: smarcet <smarcet@gmail.com> * fix(speakers): add chunk context to the all-tiers-failed log and prove send() narrowing with a control speaker Two review-thread follow-ups on #595: - SpeakerService::triggerSendEmails's per-chunk catch logged only the Throwable. JobDispatcher::withDbFallback already logs the first two tiers with summit_id / speaker_count and, since 126616d, the sync tier's failure runs ProcessSpeakersEmailRequestJob::failed() before this catch fires, but the last line should stand on its own: it now names the summit, the chunk size, the exception and the unprocessed speaker ids, with summit_id / speaker_ids / exception in the context array. - testSendSpeakersBulkEmailFilteredByMemberUserExternalId asserted only that the filtered member's speaker was in the dispatched chunk. Exact equality alone would not have proven narrowing either - the fixture summit has a single speaker with presentations - so the test now seeds a control speaker (different member, with a presentation in the summit) and requires the chunk to equal exactly [defaultSpeaker]. Mutation-verified: a filter matching every speaker (not_id==0) fails it on the control speaker. Signed-off-by: smarcet <smarcet@gmail.com> * fix(speakers): stop reporting a failed chunk as fully unprocessed and cover multi-page id resolution ProcessSpeakersEmailRequestJob::failed() said "chunk of N speaker(s) NOT processed. Unprocessed speaker ids: [...]" for every id in the chunk. The chunk is processed one speaker per transaction, so the hook's own trigger case (a worker killed mid-run) has already e-mailed and written the "already sent" proof for the speakers before the kill. An operator re-sending that list from summit-admin, which never sends should_resend, would mail those speakers twice (the DTO defaults should_resend to true). The log line and the excerpt ERROR line now say up to N of them may not have been processed, name the ids as the chunk's ids, and tell the operator to re-send with should_resend=false so the resend guard skips the ones with a proof. When the send carries a promo_code_spec the same line warns that a re-send creates a new code for every speaker in the list, because AutomaticMultiSpeakerPromoCodeStrategy generates a fresh code before the resend guard runs. The INFO line no longer claims 0 processed. Add a chunking test for the filter-based path across a page boundary: CHUNK_SIZE + 1 seeded speakers behind a first_name filter must resolve into exactly two chunks (100 + 1) covering every seeded id once, with the fixture speaker left out. This is the only path summit-admin drives and nothing exercised the do/while beyond a single page. Mutation-verified: dropping the array_merge of the pages fails it. * fix(queue): give the database fallback connection a retry_after matching redis The database connection only carried the dead Laravel-4 'expire' key, so Laravel applied its 60 s retry_after default. A ProcessSpeakersEmailRequestJob chunk that failed over to that tier (JobDispatcher::withDbFallback) and ran longer than 60 s was re-served by a sibling worker-db-fallback replica, failed on tries = 1, and the failed() hook reported a false chunk loss while the original run was still completing. Align it with the redis primary (1800, DB_QUEUE_RETRY_AFTER) and drop the unused key. * fix(speakers): route the failed-chunk excerpt through JobDispatcher::withDbFallback ProcessSpeakersEmailRequestJob::failed() dispatched the lost-chunk excerpt with a bare ::dispatch() on the default connection. A chunk runs on the database fallback worker precisely when the redis primary was down at dispatch time, so if it then failed while redis was still down the excerpt push threw, the best-effort catch swallowed it, and the operator report was lost in the one scenario it exists for. Dispatch it through JobDispatcher::withDbFallback with primaryConnection following queue.default, the same route the chunk itself takes in SpeakerService::triggerSendEmails. The try/catch stays so an excerpt failure never masks the original one. Adds a test that scripts the primary dispatch to throw and asserts the excerpt is re-dispatched on the database connection with the chunk's speaker ids; it fails against the bare dispatch (nothing captured) and passes with the fallback. * chore(config): add chunk sizes to config chore(debug): add log info * test(speakers): fix chunk-size assumptions in bulk send chunking tests SpeakerService::CHUNK_SIZE does not exist on the class; two other tests in the same file separately hardcoded a 100-based chunk size. All three now read emails.speakers_process_job_chunk_size (default 200), the same value triggerSendEmails() actually chunks by. Verified against a real dev send of 661 speakers: the app log's chunk sizes (200/200/200/61) matched the summit-admin CSV export exactly. CodeRabbit also proposed lowering the config default from 200 to 100; rejected — 200 is the value actually in effect and verified correct. * fix(speakers): stop logging access_token and raw filter PII in bulk send debug/error logs SpeakerService::triggerSendEmails's debug log json_encode()'d the full payload, including access_token - confirmed leaking a live bearer token in a real dev send today. Replaced with summit id, flow_event, speaker_ids count and whether a filter was given. ProcessSpeakersEmailRequestJob::failed()'s error log json_encode()'d the raw filter, which can carry email/full_name PII (valid speaker filter fields). New redactFilterFieldNames() logs only the filter's field names. Found and confirmed via adversarial review of CodeRabbit's full-review findings on PR #595. test(speakers): add red-green verified regression test for filter PII redaction testFailedChunkLogsFilterFieldNamesButNotTheirValues asserts the failed() error log contains the filter's field names but not an email value from it; reverting the redaction makes it fail (Mockery 0 matching calls). --------- Signed-off-by: smarcet <smarcet@gmail.com>
|
📘 OpenAPI / Swagger preview ➡️ https://OpenStackweb.github.io/summit-api/openapi/pr-593/ This page is automatically updated on each push to this PR. |
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
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.
ref:https://app.clickup.com/t/9014802374/86bbreptr
Two independent fixes found while investigating the 2026-08-31 bulk speaker email
incident (summit 73, promo code OCPSPEAKER26, killed at 500 of 683 speakers). Both
are self-contained; neither changes behaviour.
1. Quadratic hydration in the per-speaker mark-as-sent path
SpeakersPromoCodeTrait::setEmailSentfound theAssignedPromoCodeSpeakerto markthrough
$this->owners->filter().Collection::filter()goes throughAbstractLazyCollection::filter(), which callsinitialize()unconditionally, so theEXTRA_LAZYmapping on$ownersdoes not apply. The closure then dereferenced->getSpeaker()->getEmail()on every element, lazily hydrating onePresentationSpeakerper element as well.Every speaker of a bulk send shares the same promo code, so marking speaker N
hydrated N assignments plus N speakers. The cost of each speaker grew with the number
already assigned, and every speaker of the run stayed pinned in the identity map.
Over a 683 recipient send that is quadratic hydration on top of a heap that only
grows — it matches the observed decay from 57 to 10 speakers/min.
The lookup is now a targeted query. The e-mail matching mirrors
PresentationSpeaker::getEmail(), which is computed rather than mapped: the member'se-mail wins, and the registration request's is only used when the speaker has no
member.
2. Missing ORDER BY in filtered ids pagination
The default-order callback of
DoctrineRepository::getParametrizedAllIdsByPagewaschained to
$filterinstead of$order, so it only ran when no filter was given. Anyfiltered page was emitted with LIMIT/OFFSET and no ORDER BY, and MySQL makes no
promise about the order of such a result: paging through one can skip rows or return
the same row on two different pages.
The sibling
getParametrizedAllByPagealready chains the same callback to$order,which is the intended contract. This aligns the two.
The only caller is
DoctrineSpeakerRepository::getSpeakersIdsBySummit, whose callbackis the default speaker order (
e.id ASC). It never passes an explicit order, andParametrizedSendEmailssubstitutes an emptyFilterwhen none was given, so inpractice the bulk speaker send had been paging an unordered query on every run.
Tests
tests/SpeakersPromoCodeMarkSentTest.php— 7 tests. Because this is a refactor andnot a behaviour change, they were written against the previous implementation
first and passed 4/4 there; they then pass unmodified against the new one, which is
the equivalence proof. Coverage: both branches of the
getEmail()precedence, aspeaker carrying both a member and a registration request (where the precedence
actually matters), the scoping to this promo code when the same speaker is assigned
to another one, case-insensitivity, and an unknown recipient.
tests/ParametrizedAllIdsByPageOrderTest.php— 3 tests: a filtered query, anunfiltered one, and that an explicit order still suppresses the default. Red/green
verified — reverting the fix fails the filtered case.
The assertion there is on the generated DQL rather than on paged data on purpose: an
unordered query frequently happens to come back in a stable order, so a data-level
test would pass by luck against the defect and would not protect against a
regression. That reasoning is in the test's docblock so it does not get "improved"
away later.
Two mutations were run against each fix to confirm the tests have teeth. One did not
fail: removing
LOWER()from the column side still passes, because MySQL's collationis already case-insensitive — so that test is backed by the database rather than by
the SQL, and the
LOWER()is defensive.Verification
Run inside the local
summit-apicontainer.SpeakersPromoCodeMarkSentTest+ParametrizedAllIdsByPageOrderTest: 10 tests, green.(
SpeakerRepositoryTest,SubmitterRepositoryTest,SummitRegistrationPromoCodeRepositoryTest,tests/Repositories, plus the twoabove): 84 tests, 251 assertions, exit 0.
PromoCodesServiceTesthas 2 errors on the sponsor promo code path. Verifiedpre-existing: identical on a clean
main.SpeakerServiceTesthas 3 errors from fixture summit ids that do not exist in thelocal DB. Also verified pre-existing.
Not in this PR
The ticket carries the rest of the plan. Worth knowing while reviewing:
de-duplication is currently off in production (
should_resenddefaults totrueand the admin never sends the key), so retries cannot be enabled until that is fixed;
and the worker deployment sets no
terminationGracePeriodSeconds, so it defaults to30s and any SIGTERM kills a long send silently.
Summary by CodeRabbit
Bug Fixes
Tests