-
Notifications
You must be signed in to change notification settings - Fork 173
fix: add explicit Sentry error reporting for SQL syntax errors in saveTestcase tool #3615
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
…eTestcase tool Previously, SQL syntax errors were not being sent to Sentry even though they were wrapped with withSentryCaptureException. This was because these errors are intentionally thrown to trigger LangGraph's retry mechanism. Now explicitly capturing SQL syntax errors in Sentry before re-throwing them, allowing us to monitor these errors while maintaining the retry behavior. - Added Sentry.captureException() call with appropriate tags and context - Tags include errorType: 'sql_syntax_error' and toolName: 'saveTestcase' - Extra context includes the actual SQL query and parse error message Fixes #5724 🤖 Generated with [Claude Code](https://claude.ai/code) Co-Authored-By: Claude <noreply@anthropic.com>
|
WalkthroughAdds Sentry error monitoring to saveTestcaseTool: when pgParse reports a SQL syntax error, the code captures the exception with tags and extras via @sentry/node, then rethrows to preserve the existing retry flow. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Tool as saveTestcaseTool
participant Parser as pgParse
participant Sentry as Sentry SDK
participant Retry as Retry Mechanism
Tool->>Parser: parse(sql)
alt parse error
Parser-->>Tool: { error }
Tool->>Sentry: captureException(err, tags/extras)
Tool-->>Retry: throw err
Retry->>Tool: retry according to existing policy
else parse ok
Parser-->>Tool: { ast }
Tool-->>Tool: continue normal flow
end
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Possibly related PRs
Suggested reviewers
Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
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 |
The latest updates on your projects. Learn more about Vercel for GitHub.
2 Skipped Deployments
|
Updates to Preview Branch (sentry-validate-sql) ↗︎
Tasks are run on every commit but only new migration files are pushed.
View logs for this Workflow Run ↗︎. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts (1)
55-64
: Standardize Sentry tagging conventions
- Centralize tag setup in a shared helper using
Sentry.withScope
andscope.setTag
for low-cardinality metadata.- Move high-cardinality or sensitive data (e.g. full SQL, parse errors) into
extra
orcontexts
.- Adopt consistent kebab-case tag keys (e.g.
error-type
,tool-name
) and document all canonical tags in a repo guideline.
📜 Review details
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts
(2 hunks)
🧰 Additional context used
📓 Path-based instructions (4)
**/*.ts
📄 CodeRabbit inference engine (AGENTS.md)
Name utility files in camelCase (e.g., mergeSchema.ts)
Files:
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript/TSX across the codebase
**/*.{ts,tsx}
: Prefer early returns for readability
Use named exports only (no default exports)
Prefer const arrow functions over function declarations for simple utilities (e.g., const toggle = () => {})
Files:
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts
frontend/internal-packages/**
📄 CodeRabbit inference engine (AGENTS.md)
Infra and tooling (e2e, configs, storybook, agent) live under frontend/internal-packages
Files:
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts
frontend/**/*.{ts,tsx}
📄 CodeRabbit inference engine (CLAUDE.md)
Follow existing import patterns and tsconfig path aliases
Files:
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (5)
- GitHub Check: frontend-ci
- GitHub Check: frontend-lint
- GitHub Check: Supabase Preview
- GitHub Check: agent-deep-modeling
- GitHub Check: security-review
🔇 Additional comments (2)
frontend/internal-packages/agent/src/qa-agent/tools/saveTestcaseTool.ts (2)
8-8
: LGTM! Import follows naming conventions.The Sentry import follows the established pattern in the codebase and is correctly scoped to the specific functionality being added.
48-68
: Approve SQL syntax error capture in Sentry
Sentry is initialized across the project and the explicit captureException call with custom tags here aligns with existing patterns while preserving the retry flow.
🤖 Agent Deep Modeling ExecutionProcessing time: 6m 6s Command Output
RUN v3.2.4 /home/runner/work/liam/liam/frontend/internal-packages/agent (node:7630) ExperimentalWarning: WASI is an experimental feature and might change at any time ✅ [INFO] 2025-09-26T12:28:29.502Z ⏺ (pm): Planned action: Analyze the request and create a production-ready BRD with assumptions and structured functional requirements; then save the summarized business requirement and categorized functional requirements to the artifact tool. 🔧 saveRequirementsToArtifactTool(businessRequirement: "Deliver a simple,...", functionalRequirements: {12 keys}) ⎿ unknown: Requirements saved successfully to artifact 🔧 schemaDesignTool(operations: [8 items]) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) ⏺ (qa): { 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) ⏺ (qa): { 🔧 saveTestcase(testcaseWithDml: {6 keys}) 🔧 saveTestcase(testcaseWithDml: {6 keys}) ⎿ unknown: 36/48 test cases passed, 12 failed ❌ Test Case: Users can view and update their own profile fields while protecting system-managed fields1. Error:
|
Summary
Issue
Why is this change needed?
Previously, SQL syntax errors in the
validateSqlSyntax
function were not being sent to Sentry, even though the tool was wrapped withwithSentryCaptureException
. This was because these errors are intentionally thrown to trigger LangGraph's retry mechanism, and the wrapper would capture and re-throw them without special handling for monitoring purposes.Changes
@sentry/node
in saveTestcaseTool.tsSentry.captureException()
call when SQL syntax validation failserrorType: 'sql_syntax_error'
,toolName: 'saveTestcase'
)Test plan
🤖 Generated with Claude Code
Summary by CodeRabbit