-
Notifications
You must be signed in to change notification settings - Fork 5.5k
11226 bug create trigger new item in feed ends to error not a feed with my rssatom #18706
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: master
Are you sure you want to change the base?
Conversation
- Bump @pipedream/rss version to 0.5.9 - Update merge-rss-feeds action version to 1.2.9 - Add User-Agent header in rss.app.ts - Bump new-item-from-multiple-feeds source version to 1.2.8 - Bump new-item-in-feed source version to 1.2.8
…nings - Added eslint-disable comments for explicit any type in itemTs, itemKey, fetchFeed, isJSONFeed, and sortItems methods in rss.app.ts. - Updated new-item-from-multiple-feeds source to disable explicit any warning in forEach method.
- Bump @pipedream/rss version to 0.5.10 - Update merge-rss-feeds action version to 1.2.10 - Update new-item-from-multiple-feeds source version to 1.2.9 - Update new-item-in-feed source version to 1.2.9 - Update random-item-in-multiple-feeds source version to 0.2.9 - Refactor axios usage in rss.app.ts to use axios.request
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughUpdates the RSS component: adjusts HTTP request usage and headers in the app fetch logic, bumps versions for the package, one action, and three sources, adds a readOnlyHint annotation, and applies minor import order and linting comment changes. No new exported APIs; only metadata and internal request method adjustments. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
participant Src as Source/Action
participant RSS as RSS App
participant HTTP as axios.request
participant Parser as Feed Parser
Src->>RSS: fetchFeed(url)
RSS->>HTTP: GET url with headers (User-Agent, etc.)
HTTP-->>RSS: Response stream
RSS->>Parser: pipe stream and parse
Parser-->>RSS: items
RSS-->>Src: items
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (4 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
components/rss/app/rss.app.ts (1)
57-79
: Guard against missing dependency before release.This block now relies on the external
axios
package, but the module isn’t bundled anywhere unless it’s listed incomponents/rss/package.json
. Without that addition, this import will throw at runtime. Please addaxios
as a dependency in the package manifest to keep this code working.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
components/rss/actions/merge-rss-feeds/merge-rss-feeds.ts
(1 hunks)components/rss/app/rss.app.ts
(7 hunks)components/rss/package.json
(1 hunks)components/rss/sources/new-item-from-multiple-feeds/new-item-from-multiple-feeds.ts
(3 hunks)components/rss/sources/new-item-in-feed/new-item-in-feed.ts
(1 hunks)components/rss/sources/random-item-in-multiple-feeds/random-item-in-multiple-feeds.ts
(2 hunks)
🧰 Additional context used
🧬 Code graph analysis (2)
components/rss/app/rss.app.ts (1)
components/spotify/actions/get-album-tracks/get-album-tracks.mjs (1)
axios
(53-56)
components/rss/actions/merge-rss-feeds/merge-rss-feeds.ts (1)
types/src/index.ts (1)
defineAction
(402-408)
⏰ 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). (4)
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: pnpm publish
🔇 Additional comments (3)
components/rss/actions/merge-rss-feeds/merge-rss-feeds.ts (3)
8-8
: Verify the version skip from 1.2.8 to 1.2.10.The version jumps from 1.2.8 to 1.2.10, skipping 1.2.9. This pattern appears across multiple RSS components in this PR (e.g., package 0.5.8→0.5.10, sources 1.2.7→1.2.9). While this might be intentional, please confirm whether 1.2.9 was used elsewhere or if this skip is a versioning strategy.
9-13
: Good addition of readOnlyHint annotation.Adding
readOnlyHint: true
is appropriate for this action, as it only reads and merges RSS feeds without modifying any state. This correctly indicates the action's read-only nature.
24-32
: Confirm that the actual bug fix is implemented in the RSS app file.This action calls
this.rss.fetchAndParseFeed(url)
on line 27, but no logic changes are visible here. Based on the PR objectives (fixing "Not a feed" error for valid Dotclear RSS/Atom feeds) and the AI summary (HTTP request/header adjustments), the actual fix should be in therss.app.ts
file wherefetchAndParseFeed
is implemented. The version bump here indicates related changes were made, but please verify that the HTTP request logic in the app file properly handles the Dotclear feed format.
…d-ends-to-error-not-a-feed-with-my-rssatom
Resolves #11226
Summary by CodeRabbit