-
Notifications
You must be signed in to change notification settings - Fork 42
Reference for Issue #210 #214
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
Conversation
Checking the script by using "warn" as keyword as the deployment doesnt have any error now.
Parameter update
Conclusion
Conclusion
|
@a-y-a-n-das is attempting to deploy a commit to the JSON Resume Team on Vercel. A member of the Team first needs to authorize it. |
Warning Rate limit exceeded@a-y-a-n-das has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 0 minutes and 11 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (1)
WalkthroughAdds Sentry integration and a test API route to the homepage app, a Node script to migrate unresolved Sentry issues into GitHub issues, a GitHub Actions workflow to run that script, dependency updates (including @sentry/node and node-fetch override), and exposes SENTRY_DSN in turbo.json globalEnv. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor Client
participant API as /api/test-error
participant Sentry as Sentry SDK
Client->>API: HTTP request
API->>API: throw and catch error
API->>Sentry: captureException(error)
API->>Sentry: flush(timeout=2000)
API-->>Client: 500 { message: "Error sent to Sentry" }
sequenceDiagram
autonumber
participant GA as GitHub Actions
participant Script as sentry-to-github.js
participant SentryAPI as Sentry API
participant GitHub as GitHub API
GA->>Script: execute (env: SENTRY_AUTH_TOKEN, SENTRY_ORG, SENTRY_PROJECT, GITHUB_TOKEN, MY_GITHUB_REPO, SENTRY_DSN)
Script->>SentryAPI: GET /issues?query=unresolved+last24h
SentryAPI-->>Script: issues[]
loop per issue
Script->>GitHub: search issues by title
alt not found
Script->>GitHub: create issue (title, body)
GitHub-->>Script: created issue
else found
Script-->>Script: log "issue exists"
end
end
Script-->>GA: exit (success/failure)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
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 |
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: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/vercel-error-monitor.yml
(1 hunks)scripts/error-hashes.json
(1 hunks)scripts/monitor-vercel-errors.js
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/vercel-error-monitor.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
Code Review - Partial Implementation AnalysisThank you for the contribution! This is a great start on automated error monitoring. However, the current implementation has several issues that need resolution before it can be merged. Critical Issues ❌
Architecture Issues
|
Thank you for the detailed suggestions! I like the approach you recommended. Although I’m still learning, I can try to implement the Real-time error logic using Sentry/LogRocket. |
Sentry to GitHub issue
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: 9
🧹 Nitpick comments (3)
apps/homepage2/pages/api/test-error.js (1)
13-13
: Consider configurable flush timeout.A fixed 2-second timeout may be insufficient under network latency or when Sentry is experiencing degraded performance, potentially losing error reports.
Make the timeout configurable:
- await Sentry.flush(2000); // wait max 2s + const flushTimeout = parseInt(process.env.SENTRY_FLUSH_TIMEOUT || '5000', 10); + await Sentry.flush(flushTimeout);scripts/sentry-to-github.js (1)
15-15
: Hardcoded 24h window may miss important issues.The
statsPeriod=24h
filter will miss errors that occurred before the script's first run or during downtime. Consider making the time window configurable or using a cursor-based approach to track processed issues.Make the time period configurable:
+const STATS_PERIOD = process.env.SENTRY_STATS_PERIOD || '24h'; + async function fetchSentryIssues() { - const url = `https://sentry.io/api/0/projects/${SENTRY_ORG}/${SENTRY_PROJECT}/issues/?query=is:unresolved&statsPeriod=24h`; + const url = `https://sentry.io/api/0/projects/${SENTRY_ORG}/${SENTRY_PROJECT}/issues/?query=is:unresolved&statsPeriod=${STATS_PERIOD}`;.github/workflows/fetch-issue.yml (1)
60-67
: Remove excessive blank lines.Multiple consecutive blank lines reduce readability.
Per static analysis (YAMLlint), remove the excessive blank lines at the end of the file:
- - - - - - - - -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/fetch-issue.yml
(1 hunks)apps/homepage2/pages/api/test-error.js
(1 hunks)scripts/sentry-to-github.js
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/fetch-issue.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 67-67: too many blank lines (7 > 0)
(empty-lines)
🔇 Additional comments (1)
.github/workflows/fetch-issue.yml (1)
28-29
: Verify node-fetch installation approach.The command
pnpm add node-fetch -w
adds node-fetch to the workspace root, but the script runs as a standalone Node script (not as part of a pnpm workspace package). This may fail at runtime if the module resolution doesn't find node-fetch.Verify that the script can import node-fetch with this installation method. Consider alternatives:
Option 1: Use ES modules import with package.json type:
Ensure the project has"type": "module"
in package.json at the root.Option 2: Install in a way that makes it available to standalone scripts:
- - name: modules - run: pnpm add node-fetch -w - - name: Install dependencies run: pnpm installAnd add node-fetch to the appropriate package.json dependencies instead of installing it in the workflow.
Option 3: Use Node's built-in fetch (Node 18+):
Since the workflow uses Node 20, you can use the built-infetch
and remove the node-fetch dependency entirely:In
scripts/sentry-to-github.js
:-import fetch from 'node-fetch'; +(no import needed, fetch is global in Node 18+)
|
||
import Sentry from '../../sentry'; | ||
|
||
console.log("SENTRY_DSN =", process.env.SENTRY_DSN); |
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.
Remove secret logging before production.
Logging SENTRY_DSN
exposes sensitive credentials in application logs, which may be accessible to unauthorized parties or persisted in log aggregation systems.
Remove this line entirely:
-console.log("SENTRY_DSN =", process.env.SENTRY_DSN);
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
console.log("SENTRY_DSN =", process.env.SENTRY_DSN); |
🤖 Prompt for AI Agents
In apps/homepage2/pages/api/test-error.js around line 4, remove the debug log
that prints SENTRY_DSN as it exposes a secret; delete the
console.log("SENTRY_DSN =", process.env.SENTRY_DSN) line entirely and, if you
need to verify presence in development only, wrap any check in a NODE_ENV ===
"development" guard or use a secure feature-flagged debug mechanism that never
runs in production.
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: 4
♻️ Duplicate comments (3)
.github/workflows/fetch-issue.yml (3)
8-9
: Commented cron schedule prevents automated monitoring.This issue was previously flagged in past review comments. The commented schedule means the workflow only runs manually or on branch pushes, defeating the purpose of automated Sentry→GitHub issue synchronization.
Enable the schedule as suggested in previous reviews:
- #schedule: - # - cron: "0 * * * *" + schedule: + - cron: "0 */6 * * *" # Run every 6 hoursBased on past review comments.
36-46
: Environment variable name mismatch will break the script.This critical issue was previously flagged. The workflow sets
MY_GITHUB_REPO
but the script atscripts/sentry-to-github.js
expectsGITHUB_REPO
, which will cause undefined repository errors.Fix the environment variable name and remove unused SENTRY_DSN:
- name: Run Sentry to GitHub script run: | node scripts/sentry-to-github.js env: SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} SENTRY_ORG: ${{ secrets.SENTRY_ORG }} SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - MY_GITHUB_REPO: ${{ secrets.MY_GITHUB_REPO }} - SENTRY_DSN: ${{ secrets.SENTRY_DSN }} + GITHUB_REPO: ${{ secrets.MY_GITHUB_REPO }}Based on past review comments.
47-57
: CRITICAL: Debug step exposes secrets in CI logs.This critical security issue was previously flagged. Echoing secrets to logs completely undermines GitHub's secret protection and exposes sensitive credentials to anyone with access to workflow runs.
Remove this entire debug step immediately:
- - name: Debug secrets - run: | - echo "SENTRY_ORG=$SENTRY_ORG" - echo "SENTRY_PROJECT=$SENTRY_PROJECT" - echo "MY_GITHUB_REPO=$MY_GITHUB_REPO" - echo "GITHUB_TOKEN length=${#GITHUB_TOKEN}" - env: - SENTRY_ORG: ${{ secrets.SENTRY_ORG }} - SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} - MY_GITHUB_REPO: ${{ secrets.MY_GITHUB_REPO }} - GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }}Based on past review comments.
🧹 Nitpick comments (1)
.github/workflows/fetch-issue.yml (1)
58-68
: Remove excessive blank lines at end of file.The file has 8+ blank lines at the end, which violates YAML formatting conventions.
Remove the trailing blank lines:
GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - - - - - - - - - -
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/fetch-issue.yml
(1 hunks)apps/homepage2/sentry.js
(1 hunks)scripts/sentry-to-github.js
(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- scripts/sentry-to-github.js
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/fetch-issue.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 68-68: too many blank lines (8 > 0)
(empty-lines)
🔇 Additional comments (1)
apps/homepage2/sentry.js (1)
3-8
: Verify SENTRY_DSN is set to avoid silent failures.If
SENTRY_DSN
is undefined, Sentry initialization will silently fail and errors won't be captured. Consider validating required environment variables at startup.Add validation before initialization:
+if (!process.env.SENTRY_DSN) { + console.warn('SENTRY_DSN not set; Sentry monitoring is disabled.'); +} + Sentry.init({ dsn: process.env.SENTRY_DSN, tracesSampleRate: 1.0, sendDefaultPii: true, environment: process.env.NODE_ENV || "development", });Alternatively, fail fast if Sentry is mandatory:
if (!process.env.SENTRY_DSN) { throw new Error('SENTRY_DSN environment variable is required'); }
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
♻️ Duplicate comments (5)
scripts/sentry-to-github.js (4)
55-65
: Use Sentry issue ID for reliable deduplication.Title-based deduplication is fragile—if Sentry changes the error message slightly (e.g., adding timestamps, counts, or IDs), the script will create duplicate GitHub issues. Using Sentry's stable issue ID provides more reliable deduplication.
Use Sentry's issue ID for deduplication:
for (const issue of sentryIssues) { const title = issue.title; - const body = `Sentry Issue: [${title}](${issue.permalink})\n\nLast seen: ${issue.lastSeen}\nPlatform: ${issue.platform}`; + const sentryId = issue.id; + const body = `Sentry Issue: [${title}](${issue.permalink})\n\nSentry ID: ${sentryId}\nLast seen: ${issue.lastSeen}\nPlatform: ${issue.platform}`; - const exists = await githubIssueExists(title); + const exists = await githubIssueExistsBySentryId(sentryId); if (!exists) { await createGithubIssue(title, body);Add a new function that searches by Sentry ID:
async function githubIssueExistsBySentryId(sentryId) { const query = `repo:${GITHUB_REPO} is:issue is:open "Sentry ID: ${sentryId}"`; const url = `https://api.github.com/search/issues?q=${encodeURIComponent(query)}`; const res = await fetch(url, { headers: { Authorization: `token ${GITHUB_TOKEN}` }, }); if (!res.ok) { console.error(`GitHub search API error: ${res.statusText}`); return false; } const data = await res.json(); return data.total_count > 0; }
12-12
: CRITICAL: Remove authentication token logging.Line 12 logs
SENTRY_AUTH_TOKEN
to console output, which exposes the secret in CI logs. This allows anyone with log access to impersonate your Sentry account.Remove this line immediately:
const GITHUB_REPO = process.env.MY_GITHUB_REPO; // "owner/repo" format -console.log(`${SENTRY_AUTH_TOKEN}`); // Sentry issues async function fetchSentryIssues() {
6-10
: Validate required environment variables at startup.The script will fail with cryptic errors if required environment variables are missing. Adding early validation provides clearer feedback and faster failure.
Add validation before main execution:
const SENTRY_AUTH_TOKEN = process.env.SENTRY_AUTH_TOKEN; const SENTRY_ORG = process.env.SENTRY_ORG; const SENTRY_PROJECT = process.env.SENTRY_PROJECT; const GITHUB_TOKEN = process.env.GITHUB_TOKEN; const GITHUB_REPO = process.env.MY_GITHUB_REPO; // "owner/repo" format + +const required = { SENTRY_AUTH_TOKEN, SENTRY_ORG, SENTRY_PROJECT, GITHUB_TOKEN, GITHUB_REPO }; +const missing = Object.entries(required).filter(([_, v]) => !v).map(([k]) => k); +if (missing.length > 0) { + console.error(`Missing required environment variables: ${missing.join(', ')}`); + process.exit(1); +}
24-32
: Implement pagination for GitHub issue checks.The
per_page=100
limit without pagination means existing issues beyond the first page won't be detected, causing duplicate issue creation when more than 100 open issues exist.Implement pagination:
async function githubIssueExists(title) { - const url = `https://api.github.com/repos/${GITHUB_REPO}/issues?state=open&per_page=100`; - const res = await fetch(url, { - headers: { Authorization: `token ${GITHUB_TOKEN}` }, - }); - const issues = await res.json(); - console.log("GitHub API response for issues:", issues); - return issues.some((issue) => issue.title === title); + let page = 1; + while (true) { + const url = `https://api.github.com/repos/${GITHUB_REPO}/issues?state=open&per_page=100&page=${page}`; + const res = await fetch(url, { + headers: { Authorization: `token ${GITHUB_TOKEN}` }, + }); + if (!res.ok) { + console.error(`GitHub API error: ${res.statusText}`); + break; + } + const issues = await res.json(); + if (issues.length === 0) break; + if (issues.some((issue) => issue.title === title)) return true; + page++; + } + return false; }.github/workflows/fetch-issue.yml (1)
28-29
: Remove redundant node-fetch installation.Installing
node-fetch
at runtime is unnecessary and slows down the workflow. This should be declared inpackage.json
dependencies instead.Add
node-fetch
topackage.json
:"dependencies": { "@changesets/cli": "^2.27.1", "@sentry/node": "^10.17.0", + "node-fetch": "^3.3.2", "dotenv": "^16.0.3" },
Then remove the runtime installation step from the workflow:
- - name: modules - run: pnpm add node-fetch -w -
🧹 Nitpick comments (5)
.github/workflows/fetch-issue.yml (2)
45-45
: Remove unused SENTRY_DSN environment variable.The script at
scripts/sentry-to-github.js
does not useSENTRY_DSN
, making this environment variable unnecessary.Apply this diff:
env: SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} SENTRY_ORG: ${{ secrets.SENTRY_ORG }} SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} MY_GITHUB_REPO: ${{ secrets.MY_GITHUB_REPO }} - SENTRY_DSN: ${{ secrets.SENTRY_DSN }}
47-57
: Remove excessive blank lines.The file has 9+ trailing blank lines, which violates YAML linting rules.
Apply this diff to clean up the file:
MY_GITHUB_REPO: ${{ secrets.MY_GITHUB_REPO }} - - - - - - - - - - -scripts/sentry-to-github.js (3)
71-75
: Remove excessive blank lines.The file has trailing blank lines that should be removed for consistency.
Apply this diff:
} })(); - - - - -
14-21
: Add error response logging for Sentry API failures.When the Sentry API call fails, the error message only includes
statusText
, which may not provide enough context. Including the response body can help diagnose API issues.Apply this diff:
async function fetchSentryIssues() { const url = `https://sentry.io/api/0/projects/${SENTRY_ORG}/${SENTRY_PROJECT}/issues/?query=is:unresolved&statsPeriod=24h`; const res = await fetch(url, { headers: { Authorization: `Bearer ${SENTRY_AUTH_TOKEN}` }, }); - if (!res.ok) throw new Error(`Sentry fetch failed: ${res.statusText}`); + if (!res.ok) { + const errorBody = await res.text(); + throw new Error(`Sentry fetch failed: ${res.statusText}. Response: ${errorBody}`); + } return res.json(); }
35-48
: Add error response logging for GitHub issue creation failures.Similar to the Sentry API, GitHub API failures should include the response body for better diagnostics.
Apply this diff:
async function createGithubIssue(title, body) { const url = `https://api.github.com/repos/${GITHUB_REPO}/issues`; const res = await fetch(url, { method: 'POST', headers: { Authorization: `token ${GITHUB_TOKEN}`, 'Content-Type': 'application/json', }, body: JSON.stringify({ title, body }), }); - if (!res.ok) throw new Error(`GitHub issue creation failed: ${res.statusText}`); + if (!res.ok) { + const errorBody = await res.text(); + throw new Error(`GitHub issue creation failed: ${res.statusText}. Response: ${errorBody}`); + } const data = await res.json(); console.log(`Created GitHub issue #${data.number}: ${title}`); }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yaml
is excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (3)
.github/workflows/fetch-issue.yml
(1 hunks)package.json
(2 hunks)scripts/sentry-to-github.js
(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.37.1)
.github/workflows/fetch-issue.yml
[error] 1-1: wrong new line character: expected \n
(new-lines)
[warning] 57-57: too many blank lines (9 > 0)
(empty-lines)
🔇 Additional comments (1)
package.json (1)
25-25
: LGTM!The addition of
@sentry/node
dependency is appropriate for the Sentry error tracking integration introduced in this PR.
Note: Please update the current error-catching lines to use Sentry’s error handling methods. |
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.
@CodeRabbit re-review |
@a-y-a-n-das, thank you for implementing the Sentry-based approach! I'll review the updated changes, including the new Sentry configuration, GitHub Actions workflow, and the test error endpoint. ✅ Actions performedReview triggered.
|
Pull Request
Description
Partial implementation of automated error monitoring for Vercel deployments.
Initially, I was fetching logs from a sample file for testing.
When running against a Vercel deployment, it became clear that Vercel only provides runtime logs, which are ephemeral.
As a result, continuous monitoring would be required to catch errors in real-time.
This PR includes the partial script for reference and as a starting point for future implementation.
Related Issues
Relates to #210
Changes Made
Type of Change
Testing
Test Details:
Quality Checklist
pnpm test
)pnpm lint
)pnpm prettier
)find . -name "*.ts" -o -name "*.tsx" -o -name "*.js" | xargs wc -l | awk '$1 > 150'
)pnpm audit
)any
unless necessary)Performance Impact
Details:
Breaking Changes
Details:
Screenshots/Demos
Additional Notes
Additional Notes
Vercel runtime logs are ephemeral; historical logs are not available.
Full continuous monitoring and real-time error detection will require additional implementation.
For Reviewers:
Summary by CodeRabbit
New Features
Chores