Fixes issue #210 in a better solution #228
Conversation
Checking the script by using "warn" as keyword as the deployment doesnt have any error now.
Parameter update
Conclusion
Conclusion
Sentry to GitHub issue
|
|
@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. |
|
Important Review skippedReview was skipped due to path filters ⛔ Files ignored due to path filters (1)
CodeRabbit blocks several paths by default. You can override this behavior by explicitly including those paths in the path filters. For example, including You can disable this status message by setting the WalkthroughAdds Sentry initialization and a test API route, a script to sync unresolved Sentry issues to GitHub, a GitHub Actions workflow to run that script, new runtime deps (@sentry/node, node-fetch), and exposes SENTRY_DSN in turbo.json. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Client
participant API as /api/test-error
participant Sentry
Client->>API: GET /api/test-error
API->>API: throw error
API->>Sentry: captureException(err)
API->>Sentry: flush(timeout=2000ms)
API-->>Client: 500 { sent: true }
sequenceDiagram
autonumber
participant Trigger as Scheduler/Push/Manual
participant GA as GitHub Actions
participant Script as sentry-to-github.js
participant SentryAPI as Sentry API
participant GHAPI as GitHub API
Trigger->>GA: start fetch-issue-sentry
GA->>Script: run Node (env: SENTRY_*, GITHUB_*, MY_GITHUB_REPO)
Script->>SentryAPI: GET unresolved issues (recent)
SentryAPI-->>Script: issues[]
loop per issue
Script->>GHAPI: GET open issues (filter/title)
GHAPI-->>Script: openIssues[]
alt no matching title
Script->>GHAPI: POST create issue (title, body with link/metadata)
GHAPI-->>Script: createdIssue
else exists
Script->>Script: skip
end
end
Script-->>GA: exit (logs)
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 9
🧹 Nitpick comments (4)
apps/homepage2/sentry.js (1)
3-8: Validate SENTRY_DSN before initialization.The code initializes Sentry without checking if
SENTRY_DSNis set. If the DSN is missing, Sentry may fail silently or produce unclear errors.Apply this diff to add validation:
+if (!process.env.SENTRY_DSN) { + console.warn('SENTRY_DSN not set, Sentry will not be initialized'); +} else { Sentry.init({ dsn: process.env.SENTRY_DSN, tracesSampleRate: 1.0, sendDefaultPii: true, environment: process.env.NODE_ENV || "development", }); +}.github/workflows/fetch-issue.yml (1)
44-55: Remove excessive blank lines.There are 10+ blank lines at the end of the file, which violates YAML formatting standards.
Apply this diff:
SENTRY_DSN: ${{ secrets.SENTRY_DSN }} - - - - - - - - - - - -scripts/sentry-to-github.js (2)
30-30: Remove verbose logging of all GitHub issues.Logging all GitHub issues to the console can be very verbose and expose issue details unnecessarily. Consider removing this log statement or making it conditional on a debug flag.
Apply this diff:
- console.log("GitHub API response for issues:", issues);
24-32: Strengthen duplicate detection logic.The current duplicate check uses simple title matching (
issue.title === title), which could produce false positives if two different errors have similar titles. Consider incorporating additional metadata like the Sentry issue ID or permalink in the GitHub issue body and checking for that.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (6)
.github/workflows/fetch-issue.yml(1 hunks)apps/homepage2/pages/api/test-error.js(1 hunks)apps/homepage2/sentry.js(1 hunks)package.json(2 hunks)scripts/sentry-to-github.js(1 hunks)turbo.json(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] 55-55: too many blank lines (10 > 0)
(empty-lines)
🔇 Additional comments (3)
turbo.json (1)
19-19: LGTM!The addition of
SENTRY_DSNto the global environment variables is correct and necessary for the Sentry integration across the workspace..github/workflows/fetch-issue.yml (1)
5-7: Verify the branch trigger is intentional.The workflow triggers on pushes to
sentry-to-github-issuebranch. Typically, such workflows run onmainormaster. Confirm this is the intended behavior and not a leftover from testing.apps/homepage2/pages/api/test-error.js (1)
6-16: Restrict test-error endpoint in production.
Wrap this handler in a NODE_ENV check (e.g., only in development) or remove it from prod builds to prevent unintended exposure.
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (2)
scripts/sentry-to-github.js (2)
13-20: Implement pagination for Sentry API calls.The function fetches only the first page of results. If there are more unresolved issues than the default page size (typically 100), subsequent issues will be missed and won't be synced to GitHub.
Consult the Sentry API documentation for pagination parameters (likely using cursor-based pagination or page/limit parameters) and implement a loop to fetch all pages until no more results are returned.
23-31: Implement pagination for GitHub API calls.The function only checks the first 100 open issues. If the repository has more than 100 open issues, the duplicate detection will fail and create duplicate GitHub issues.
A previous review provided a detailed implementation for pagination. Please implement that fix to check all open issues across all pages.
🧹 Nitpick comments (3)
.github/workflows/fetch-issue.yml (1)
25-31: Consider using pnpm/action-setup for better performance.Installing pnpm globally and running
pnpm installwithout caching will reinstall dependencies on every run. Consider usingpnpm/action-setupwith cache configuration for faster builds.Apply this diff:
- - name: Setup pnpm - run: npm install -g pnpm + - name: Setup pnpm + uses: pnpm/action-setup@v2 + with: + version: 8 - name: Install dependencies run: pnpm install env: NODE_AUTH_TOKEN: ${{ secrets.GITHUB_TOKEN }}Then add caching after setup-node:
- name: Setup Node.js uses: actions/setup-node@v4 with: node-version: 20 + cache: 'pnpm'scripts/sentry-to-github.js (2)
29-29: Remove or reduce verbose logging.Logging the entire GitHub API response could expose sensitive information and create very large logs if there are many issues. Consider removing this log or replacing it with a count.
Apply this diff:
- console.log("GitHub API response for issues:", issues); + console.log(`Checked ${issues.length} GitHub issues for duplicates`);
54-64: Consider rate limiting when processing many issues.The script processes issues sequentially, which is good for avoiding rate limits. However, if there are many issues, consider adding:
- Rate limiting awareness (check for 429 responses)
- Exponential backoff on errors
- Progress logging every N issues
This is optional for now but recommended if the script will process large volumes.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 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)
🚧 Files skipped from review as they are similar to previous changes (1)
- apps/homepage2/pages/api/test-error.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] 54-54: too many blank lines (11 > 0)
(empty-lines)
🔇 Additional comments (2)
scripts/sentry-to-github.js (2)
1-2: LGTM!The ESM import is correct for node-fetch v3.x.
34-47: LGTM!The function correctly creates GitHub issues with appropriate error handling.
| @@ -0,0 +1,54 @@ | |||
| name: fetch-issue-sentry | |||
There was a problem hiding this comment.
Fix YAML newline character.
The file uses an incorrect newline character. Ensure the file uses Unix-style line endings (\n) instead of Windows-style (\r\n).
You can fix this with:
#!/bin/bash
# Convert line endings to Unix format
dos2unix .github/workflows/fetch-issue.yml
# Or with sed
sed -i 's/\r$//' .github/workflows/fetch-issue.yml🧰 Tools
🪛 YAMLlint (1.37.1)
[error] 1-1: wrong new line character: expected \n
(new-lines)
🤖 Prompt for AI Agents
.github/workflows/fetch-issue.yml lines 1-1: the file currently contains
Windows-style CRLF line endings; convert the file to Unix-style LF line endings.
Open the file in your editor or use a line-ending conversion tool to remove
carriage returns so every newline is '\n', then save and commit the change;
optionally add or update a .gitattributes entry (e.g., mark YAML as text with
LF) to ensure future commits preserve Unix line endings.
| 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 |
There was a problem hiding this comment.
Add validation for required environment variables.
The script reads environment variables without validation. If any are undefined, the script will fail with unclear errors during API calls. Add validation to fail fast with clear error messages.
Add validation after line 10:
const GITHUB_REPO = process.env.MY_GITHUB_REPO; // "owner/repo" format
+
+// Validate required environment variables
+const requiredEnvVars = [
+ 'SENTRY_AUTH_TOKEN',
+ 'SENTRY_ORG',
+ 'SENTRY_PROJECT',
+ 'GITHUB_TOKEN',
+ 'MY_GITHUB_REPO'
+];
+
+for (const varName of requiredEnvVars) {
+ if (!process.env[varName]) {
+ console.error(`Error: Missing required environment variable ${varName}`);
+ process.exit(1);
+ }
+}📝 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.
| 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 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 | |
| // Validate required environment variables | |
| const requiredEnvVars = [ | |
| 'SENTRY_AUTH_TOKEN', | |
| 'SENTRY_ORG', | |
| 'SENTRY_PROJECT', | |
| 'GITHUB_TOKEN', | |
| 'MY_GITHUB_REPO' | |
| ]; | |
| for (const varName of requiredEnvVars) { | |
| if (!process.env[varName]) { | |
| console.error(`Error: Missing required environment variable ${varName}`); | |
| process.exit(1); | |
| } | |
| } |
🤖 Prompt for AI Agents
In scripts/sentry-to-github.js around lines 6 to 10, the environment variables
are read but not validated; add a fail-fast validation block immediately after
line 10 that checks SENTRY_AUTH_TOKEN, SENTRY_ORG, SENTRY_PROJECT, GITHUB_TOKEN,
and GITHUB_REPO, and if any are missing log a clear error identifying which
variable(s) are undefined and exit the process with a non-zero code (e.g.,
console.error with the variable names and process.exit(1)); implement this by
collecting required names, filtering to the missing ones, and failing when the
resulting list is non-empty.
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
||
|
|
There was a problem hiding this comment.
Remove excessive blank lines at end of file.
The file has 6 trailing blank lines. Clean up by removing all but one blank line at the end.
🤖 Prompt for AI Agents
In scripts/sentry-to-github.js around lines 69 to 75 there are 6 trailing blank
lines; remove the extra blank lines so the file ends with a single newline (one
blank line at EOF) to clean up whitespace while preserving a single newline at
the end of file.
1 participant
Pull Request
Description
This PR implements a comprehensive solution for Issue #210.
While a previous PR attempted a partial fix, this update fully addresses the problem. The previous implementation only partially addressed the problem, while this update ensures that:
All existing GitHub issues are checked across all pages.
Duplicate issues are correctly avoided.
New errors from Sentry are reliably reported as GitHub issues.
Related Issues
Closes #
Relates to #210
Changes Made
Note: Please update the current error-catching lines to use Sentry’s error handling methods.
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)anyunless absolutely necessary)Performance Impact
Details:
Breaking Changes
Details:
Screenshots/Demos
Additional Notes
For Reviewers:
Summary by CodeRabbit
New Features
Chores