Update RSS component versions and add User-Agent header #18704
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
|
The latest updates on your projects. Learn more about Vercel for GitHub. 2 Skipped Deployments
|
WalkthroughAdded a User-Agent header, enabled full HTTP responses, and introduced explicit HTTP-status-based error handling in the RSS app fetch logic; added readOnlyHint metadata and bumped versions for an RSS action and multiple RSS sources; package version incremented. Changes
Sequence Diagram(s)sequenceDiagram
participant Source as RSS Source
participant App as RSS App (fetchFeed)
participant HTTP as HTTP Endpoint
Source->>App: request feed URL
App->>HTTP: fetch(url, headers: User-Agent, returnFullResponse:true)
alt HTTP 200
HTTP-->>App: full response (200, body)
App-->>Source: parsed feed items
else HTTP 404
HTTP-->>App: response 404
App-->>Source: throw ConfigurationError (404)
else HTTP 429
HTTP-->>App: response 429
App-->>Source: throw ConfigurationError (429)
else HTTP 4xx (other)
HTTP-->>App: response 4xx
App-->>Source: throw ConfigurationError (4xx)
else HTTP >=500
HTTP-->>App: response >=500
App-->>Source: throw ConfigurationError (5xx)
end
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Poem
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
⏰ 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)
🔇 Additional comments (2)
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: 0
🧹 Nitpick comments (1)
components/rss/app/rss.app.ts (1)
62-62: LGTM! User-Agent header resolves feed blocking issue.The User-Agent header addition directly addresses the issue with feeds that require proper identification (like lesechos.fr). The format follows standard bot naming conventions.
Consider whether the hardcoded version "v1.0" should be dynamically sourced from package.json to stay synchronized with releases. However, a static version is acceptable if you prefer stable identification across package updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
components/rss/actions/merge-rss-feeds/merge-rss-feeds.ts(1 hunks)components/rss/app/rss.app.ts(1 hunks)components/rss/package.json(1 hunks)components/rss/sources/new-item-from-multiple-feeds/new-item-from-multiple-feeds.ts(2 hunks)components/rss/sources/new-item-in-feed/new-item-in-feed.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
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: pnpm publish
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
- GitHub Check: Lint Code Base
🔇 Additional comments (4)
components/rss/sources/new-item-in-feed/new-item-in-feed.ts (1)
10-10: LGTM! Version bump aligns with package updates.The version increment appropriately reflects the underlying changes to the RSS app (User-Agent header addition).
components/rss/package.json (1)
3-3: LGTM! Package version bump is appropriate.The minor version increment correctly reflects the addition of the User-Agent header and metadata improvements.
components/rss/sources/new-item-from-multiple-feeds/new-item-from-multiple-feeds.ts (1)
2-2: LGTM! Import order and version updates are correct.The import reordering has no semantic impact, and the version bump is consistent with the other RSS source components.
Also applies to: 11-11
components/rss/actions/merge-rss-feeds/merge-rss-feeds.ts (1)
2-2: LGTM! Metadata improvements enhance action semantics.The addition of
readOnlyHint: truecorrectly indicates that this action only reads RSS feeds without modifying external state. The version bump and import adjustments are appropriate.Also applies to: 8-8, 12-12
…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.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
components/rss/app/rss.app.ts (2)
62-62: Consider using a dynamic version for the User-Agent header.The User-Agent is currently hardcoded as
v1.0. If this version needs to stay in sync with the package version, consider deriving it frompackage.jsonto avoid maintenance overhead.Example approach:
import { version } from "../package.json"; // In the header: "User-Agent": `@PipedreamHQ/pipedream v${version}`,Alternatively, if
v1.0is intentional and represents an API version rather than the package version, consider adding a comment to clarify this.
33-33: Consider improving type safety in a follow-up.Multiple
anytype suppressions were added throughout this file. While these may be acceptable given the dynamic nature of RSS/JSON feeds and the untypedfeedparserlibrary, some improvements could be made:
- Line 56:
fetchFeedcould returnPromise<{ data: ReadStream; contentType: string }>instead ofPromise<any>- Line 123:
isJSONFeedparameter could be typed as{ contentType?: string }instead ofany- Lines 33, 43: Consider defining a union type for RSS/JSON feed items instead of
Item | anySince this PR focuses on bug fixes, these type improvements can be deferred.
Also applies to: 43-43, 56-56, 90-90, 123-123, 160-160
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
components/rss/app/rss.app.ts(6 hunks)components/rss/sources/new-item-from-multiple-feeds/new-item-from-multiple-feeds.ts(3 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- components/rss/sources/new-item-from-multiple-feeds/new-item-from-multiple-feeds.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). (4)
- GitHub Check: pnpm publish
- GitHub Check: Lint Code Base
- GitHub Check: Publish TypeScript components
- GitHub Check: Verify TypeScript components
🔇 Additional comments (1)
components/rss/app/rss.app.ts (1)
69-75: LGTM: Comprehensive HTTP status error handling.The explicit error handling for different HTTP status codes is well-implemented:
- 404: Clear message about non-existent URL
- 429: Helpful guidance about rate limiting
- 5xx: Appropriate server error message
- Other 4xx: Catch-all for client errors
The error messages are user-friendly and actionable. Combined with the User-Agent header addition, this should resolve the reported issue.
vunguyenhung
deleted the
14298-bug-rss-source-does-not-work-with-httpsserviceslesechosfrrssles-echos-entreprisesxml
branch
3 participants
Resolves #14298
Summary by CodeRabbit
New Features
Bug Fixes
Chores