-
Notifications
You must be signed in to change notification settings - Fork 2
fix: throw on jira request errors #168
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
close IDP-1295
🦋 Changeset detectedLatest commit: d68d197 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
WalkthroughIntroduces a patch changeset for @alauda/doom, bumps four dependencies, and updates resolve-release.ts to use isProduction for environment checks, adjust logging to include JQL, and alter Jira error handling to throw in production and log/return in non-production. No public API changes. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User as Build/Plugin Runner
participant Plugin as resolve-release.ts
participant Env as Environment
participant Jira as Jira API
participant Log as Logger
User->>Plugin: Invoke resolveRelease(...)
Plugin->>Env: isProduction()
alt Production
Plugin->>Log: info("Fetching release notes", { jql, releaseQuery })
Plugin->>Plugin: Validate Jira creds
alt Missing creds
Plugin-->>User: throw Error("Missing Jira credentials")
else Creds ok
Plugin->>Jira: GET issues (JQL)
alt Request fails
Plugin-->>User: throw Error(err)
else Success
Plugin-->>User: return notes
end
end
else Non-production
Plugin->>Log: info("Fetching release notes", { jql, releaseQuery })
Plugin->>Plugin: Validate Jira creds
alt Missing creds
Plugin->>Log: warn("Missing Jira credentials")
Plugin-->>User: return
else Creds ok
Plugin->>Jira: GET issues (JQL)
alt Request fails
Plugin->>Log: error(err)
Plugin-->>User: return
else Success
Plugin-->>User: return notes
end
end
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Poem
✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
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.
Pull Request Overview
This PR improves error handling for Jira requests by ensuring errors are thrown in production environments instead of being silently ignored.
- Replaces local
isCIutility with@rspress/shared'sisProduction()function for better environment detection - Updates error handling to throw exceptions in production when Jira requests fail
- Updates package dependencies to newer patch versions
Reviewed Changes
Copilot reviewed 3 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
| packages/doom/src/plugins/replace/resolve-release.ts | Switches from isCI to isProduction() and adds error throwing in production for failed Jira requests |
| packages/doom/package.json | Updates dependencies to newer patch versions |
| .changeset/lemon-pears-repair.md | Adds changeset entry documenting the fix |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
commit: |
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 (3)
.changeset/lemon-pears-repair.md (1)
5-5: Add Jira key to the changeset for traceabilityIf policy allows, reference the Jira ticket to ease cross-linking in release notes.
-fix: throw on jira request errors +fix: throw on jira request errors (IDP-1295)packages/doom/src/plugins/replace/resolve-release.ts (2)
127-129: Downgrade verbose JQL to debug or truncateJQL can be long/noisy. Consider logging at debug or truncating for readability.
- logger.info( - `Fetching release notes for query \`${cyan(releaseQuery)}\`, JQL: \`${cyan(jql)}\``, - ) + const jqlPreview = jql.length > 400 ? jql.slice(0, 400) + '…' : jql + ;(logger.debug ?? logger.info)( + `Fetching release notes for \`${cyan(releaseQuery)}\`, JQL: \`${cyan(jqlPreview)}\``, + )
145-147: Preserve context when rethrowingWrap with a richer message and use
causeto keep the original stack (Node >=18 supports it).- if (isProduction()) { - throw err - } + if (isProduction()) { + throw new Error( + `Failed to fetch Jira release notes for query: ${releaseQuery}`, + { cause: err instanceof Error ? err : undefined }, + ) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
⛔ Files ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (3)
.changeset/lemon-pears-repair.md(1 hunks)packages/doom/package.json(3 hunks)packages/doom/src/plugins/replace/resolve-release.ts(4 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: JounQin
PR: alauda/doom#30
File: src/runtime/components/K8sCrd.tsx:5-5
Timestamp: 2025-05-26T09:09:21.339Z
Learning: The alauda/doom project uses yarn v4 for dependency management, which handles package exports and module resolution correctly, so imports like `rspress/core/theme` work without any ESLint errors.
📚 Learning: 2025-05-29T10:22:30.004Z
Learnt from: JounQin
PR: alauda/doom#34
File: src/cli/load-config.ts:290-0
Timestamp: 2025-05-29T10:22:30.004Z
Learning: In the alauda/doom project, the team prefers throwing descriptive errors for invalid configuration rather than using optional chaining or graceful fallbacks. This fail-fast approach helps catch configuration issues early during development.
Applied to files:
.changeset/lemon-pears-repair.md
📚 Learning: 2025-05-29T10:23:34.965Z
Learnt from: JounQin
PR: alauda/doom#34
File: src/cli/load-config.ts:290-0
Timestamp: 2025-05-29T10:23:34.965Z
Learning: In the alauda/doom project, JounQin prefers concise error handling over verbose validation. While they prefer throwing on invalid configuration rather than graceful fallbacks, they don't want overly detailed validation with multiple explicit checks and error messages.
Applied to files:
.changeset/lemon-pears-repair.md
📚 Learning: 2025-05-26T09:09:21.339Z
Learnt from: JounQin
PR: alauda/doom#30
File: src/runtime/components/K8sCrd.tsx:5-5
Timestamp: 2025-05-26T09:09:21.339Z
Learning: The alauda/doom project uses yarn v4 for dependency management, which handles package exports and module resolution correctly, so imports like `rspress/core/theme` work without any ESLint errors.
Applied to files:
packages/doom/package.json
📚 Learning: 2025-06-06T07:08:55.881Z
Learnt from: JounQin
PR: alauda/doom#69
File: package.json:101-101
Timestamp: 2025-06-06T07:08:55.881Z
Learning: When a package has peer dependencies that require TypeScript (like `eslint-react/eslint-plugin`), TypeScript should be moved from devDependencies to dependencies to satisfy the peer dependency requirement.
Applied to files:
packages/doom/package.json
📚 Learning: 2025-05-29T16:25:28.086Z
Learnt from: JounQin
PR: alauda/doom#40
File: src/plugins/sitemap/index.ts:7-7
Timestamp: 2025-05-29T16:25:28.086Z
Learning: In rspress/shared v2.0.0-beta.8, the '/logger' export is available in the package exports field, so imports like `import { logger } from 'rspress/shared/logger'` are valid even if ESLint shows resolution errors. This is used throughout the codebase in files like src/cli/translate.ts, src/cli/load-config.ts, src/utils/git.ts, and src/plugins/sitemap/index.ts.
Applied to files:
packages/doom/package.jsonpackages/doom/src/plugins/replace/resolve-release.ts
📚 Learning: 2025-05-12T11:11:41.048Z
Learnt from: JounQin
PR: alauda/doom#9
File: src/cli/load-config.ts:27-27
Timestamp: 2025-05-12T11:11:41.048Z
Learning: The yoctocolors package is used in the codebase for colored terminal output, with the cyan function imported in src/cli/load-config.ts.
Applied to files:
packages/doom/src/plugins/replace/resolve-release.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). (6)
- GitHub Check: Build and Lint with Node.js 20 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 22 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 18 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 24 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 18 on ubuntu-latest
- GitHub Check: Build and Lint with Node.js 20 on ubuntu-latest
🔇 Additional comments (3)
packages/doom/package.json (1)
45-45: Deps bumps look safe; ensure lockfile updatedLooks good. Please make sure the Yarn v4 lockfile is updated and deduped in this PR to avoid CI drift.
Also applies to: 58-58, 96-96, 99-99
packages/doom/src/plugins/replace/resolve-release.ts (2)
109-114: Production-only hard fail on missing Jira creds — good alignment with PR goalThrowing in production and warning in non-prod matches “fail-fast” preference noted for this repo.
2-2: ConfirmisProductiontype in @rspress/shared
The repo doesn’t include its definition—verify in your dependency whetherisProductionis a boolean or a function. If it’s a boolean, remove all()from its usages; if it’s a function, leave the calls as-is.
close IDP-1295
Summary by CodeRabbit
Bug Fixes
Chores