@zen-agentchecks…zen/respect-logtoerror-trigger-manage-7q43nm → master3 files · +76 −23review: @sohamganatrareview: @kaavee315review: @lingalarahul7review: @anshugarg15updated 1mo agoApollo's PATCH/DELETE handlers under `/api/v3/trigger_instances/manage/{id}` (and the shared `manageTriggerInstanceStatus` helper) emit an unconditional `logger.error()` before delegating to `errorToHTTPException()`. For expected client-side failures — notably `TriggerInstance_TriggerInstanceGone` (HTTP 410, `logToError:false`) — this dumps the full error envelope into apollo-logs at ERROR severity, polluting the error budget. The 410 fires whenever a user deletes a trigger (or its connection/auth-config/project) and then re-issues a manage request, so it is routine, not an incident.
`errorToHTTPException` already routes severity by status code via `logger.logByStatusCode` (4xx → `warn`, 5xx → `error`) with PII sanitization, so the in-handler `logger.error` calls were both noisy and duplicative. This PR removes them. 5xx still surfaces at ERROR via the shared path.
Observed in production (last 6h, apollo-logs index): 4 occurrences of `"Error getting trigger instance: ..."` with `"slug":"TriggerInstance_TriggerInstanceGone","logToError":false,"statusCode":410`. The Datadog-side noise grouping ranked TriggerInstanceGone among the top noisy patterns alongside transient network/refresh errors that are also out-of-scope to "fix."
Origin: cron-dd-errors / zen-cron-ae7f210a91c5 Session: https://zen-api-production-4c98.up.railway.app/dashboard/#/chat/zen-cron-ae7f210a91c5
Based on git blame analysis of 3 file(s):
| Contributor | Contribution | Files |
|---|---|---|
| Zen | 36% | 1 |
| lingalarahul7 |
| 30% |
| 2 |
| anshugarg15 | 14% | 1 |
| Himanshu Dixit | 12% | 1 |
| devin-ai-integration[bot] | 8% | 1 |
Recommend Zen and anshugarg15. Zen made the most recent edit (today) to the test file and can validate test expectations, while anshugarg15 updated the manageTriggerInstance implementation about two weeks ago and can review the logic and behavioral changes.
🤖 Based on git blame with recency weighting (recent edits count more).
:white_check_mark: All modified and coverable lines are covered by tests.
| Flag | Coverage Δ | |
|---|---|---|
| e2e-tests | 5.82% <ø> (+<0.01%) | :arrow_up: |
| self-hosted-tests | ? | |
| thermos-service-isolation-tests | ? | |
| thermos-unit-tests | ? | |
| unit-tests | 58.76% <ø> (+<0.01%) | :arrow_up: |
Flags with carried forward coverage won't be shown. Click here to find out more.
| Files with missing lines | Coverage Δ | |
|---|---|---|
| ...s/apollo/src/lib/triggers/manageTriggerInstance.ts | 50.60% <ø> (+7.12%) | :arrow_up: |
| .../src/pages/api/v3/trigger_instances/manage/[id].ts | 0.00% <ø> (ø) |
... and 320 files with indirect coverage changes
