fix: remove unsupported incident.url variable from alert documentation#32
fix: remove unsupported incident.url variable from alert documentation#32
Conversation
…docs cleanup ## Root cause fix Every call to getSecret() instantiated a new SecretManagerServiceClient, opening a gRPC channel that was never closed. ~169 requests over 28 minutes → 488 MiB OOM. Fix: singleton client at module level. Secret values are NOT cached (would break rotation on warm instances). ## Governor address guard QuickNode evmAbiFilter ignores the 'contracts' field in templateArgs (silently, empirically confirmed). Added MENTO_GOVERNOR_ADDRESS guard in process-event.ts so events from other OZ Governor contracts on Celo don't produce false notifications. ## Deploy script fixes - Global TMP_FILES array + single EXIT trap (fixes trap overwrite on multiple deploy_webhook calls) - Code comment explaining why templateArgs uses 'abiJson' not 'abi' (spec says 'abi', live evmAbiFilterGo endpoint requires 'abiJson') - Comment explaining hardcoded webhook UUIDs and how to update them - Updated Terraform filter_function blobs to current trimmed ABIs ## Docs & cleanup - Delete bin/update-quicknode-filter.js (zombie script, never worked) - Remove dead dev:webhook:* npm scripts - Consolidate quicknode.tf comment: UPDATE=script, RECREATE=Terraform - Rewrite README + ADDING_EVENTS.md to reflect actual deploy workflow ## Tests - 5 unit tests for get-secret.ts (singleton behavior, no caching, errors) - 7 unit tests for process-event.ts governor address guard (canonical address, wrong address, all 4 event types, mixed-case, MedianUpdated) - npm test now runs vitest before integration tests
GCP Cloud Monitoring no longer supports the ${incident.url} variable in
alert policy documentation templates — it renders as a literal error:
'Unrecognized variable: incident.url'.
Replace with a direct link to the Cloud Logging query for recent errors.
![cursor[bot]](https://avatars.githubusercontent.com/in/1210556?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR updates QuickNode webhook/filter docs and scripts, adds unit tests, and introduces runtime guards for governor-address filtering. The direction is good, but there are two type-safety regressions introduced in this change set, including one that currently breaks the production build.
src/utils/is-valid-quicknode-payload.ts
- 🔴 Critical
line 17(also18-22): the new(event as Record<string, unknown>)casts are invalid understricttyping and fail compilation.npm run buildnow fails withTS2352in this file, so this is a merge blocker.
src/events/__tests__/process-event.test.ts
- 🟡 Important
line 45:valuesis set to astring("0") butProposalCreatedEvent.valuesis typed asbigint | readonly bigint[]. This introduces a type mismatch in the new test fixture and fails type-checking with the root tsconfig.
Final verdict: 🚫 Needs changes
Sent by Cursor Automation: PR Review
| "logIndex" in event && | ||
| "name" in event && | ||
| "transactionHash" in event | ||
| typeof (event as Record<string, unknown>).address === "string" && |
There was a problem hiding this comment.
🔴 Critical: This cast pattern now breaks compilation (TS2352) because event is inferred as QuicknodeEvent in this callback and is not safely castable to Record<string, unknown> in strict mode. This regression currently makes npm run build fail. Please rewrite this guard so the callback variable is treated as unknown before narrowing (or use a dedicated type guard helper) instead of asserting QuicknodeEvent to Record<string, unknown>.
| signatures: "", | ||
| startBlock: BigInt(1), | ||
| targets: "0x1234567890123456789012345678901234567890", | ||
| values: "0", |
There was a problem hiding this comment.
🟡 Important: values is set to "0" (string), but ProposalCreatedEvent.values is typed as bigint | readonly bigint[]. This makes the new test fixture type-inaccurate and fails type-checking with the root tsconfig. Use BigInt(0) (or 0n) to keep the fixture aligned with the event contract.
|
Closing in favor of conflict-free version |
1 participant
Problem
Alert notifications were showing:
GCP Cloud Monitoring dropped support for
${incident.url}in alert policy documentation templates.Fix
Replace the broken variable with a direct link to the Cloud Logging query for recent errors. The existing logs URL at the bottom of the alert message already provides direct access to error logs.