-
-
Notifications
You must be signed in to change notification settings - Fork 1k
fix: Accept more permissive RSS feed content types and Fix User-Agent key #2353
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: main
Are you sure you want to change the base?
Conversation
Some feeds are Content-Type application/xml only and will respond with a 406 error to responses with a header of content type application/rss+xml. This change allows for the more permissive content types application/xml and text/xml to be accepted Also fixes UserAgent with correct User-Agent
WalkthroughUpdated HTTP request headers in the feed worker: corrected header name from Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes 🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. 📜 Recent review detailsConfiguration used: Organization UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used📓 Path-based instructions (3)**/*.{ts,tsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx,json,css,md}📄 CodeRabbit inference engine (AGENTS.md)
Files:
**/*.{ts,tsx,js,jsx}📄 CodeRabbit inference engine (AGENTS.md)
Files:
🔇 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 |
Greptile SummaryThis PR fixes two issues in the RSS feed worker: corrects the HTTP header key from Key Changes:
Confidence Score: 5/5
Important Files Changed
Sequence DiagramsequenceDiagram
participant FeedWorker as Feed Worker
participant fetchWithProxy as fetchWithProxy
participant FeedServer as Feed Server
participant Parser as RSS Parser
FeedWorker->>fetchWithProxy: "Request feed with User-Agent and Accept headers"
fetchWithProxy->>FeedServer: "HTTP GET with content negotiation"
alt Server returns application/rss+xml
FeedServer-->>fetchWithProxy: "200 OK (application/rss+xml)"
else Server returns application/xml
FeedServer-->>fetchWithProxy: "200 OK (application/xml)"
else Server returns text/xml
FeedServer-->>fetchWithProxy: "200 OK (text/xml)"
end
fetchWithProxy-->>FeedWorker: "Response with XML content"
FeedWorker->>FeedWorker: "Validate content-type includes xml"
FeedWorker->>Parser: "Parse XML feed"
Parser-->>FeedWorker: "Parsed feed items"
FeedWorker->>FeedWorker: "Process entries and create bookmarks"
|
Greptile found no issues!From now on, if a review finishes and we haven't found any issues, we will not post anything, but you can confirm that we reviewed your changes in the status check section. This feature can be toggled off in your Code Review Settings by deselecting "Create a status check for each PR". |
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 (1)
apps/workers/workers/feedWorker.ts (1)
161-161: Good expansion of accepted content types.The Accept header now properly includes
application/xmlandtext/xmlwith quality values, which will prevent 406 errors from feeds that return these content types instead of the more specificapplication/rss+xml. The quality value syntax is correct and establishes proper preference ordering.Note: There appears to be trailing whitespace after the closing quote. Please run
pnpm lint:fixto ensure formatting is consistent with project standards.
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
apps/workers/workers/feedWorker.ts
🧰 Additional context used
📓 Path-based instructions (3)
**/*.{ts,tsx}
📄 CodeRabbit inference engine (AGENTS.md)
Use TypeScript for type safety in all source files
Files:
apps/workers/workers/feedWorker.ts
**/*.{ts,tsx,js,jsx,json,css,md}
📄 CodeRabbit inference engine (AGENTS.md)
Format code using Prettier according to project standards
Files:
apps/workers/workers/feedWorker.ts
**/*.{ts,tsx,js,jsx}
📄 CodeRabbit inference engine (AGENTS.md)
Lint code using oxlint and fix issues with
pnpm lint:fix
Files:
apps/workers/workers/feedWorker.ts
🔇 Additional comments (1)
apps/workers/workers/feedWorker.ts (1)
159-160: Excellent fix for the User-Agent header.The header key is now correctly spelled as
"User-Agent"(with hyphen) per HTTP standards. The previousUserAgentkey was non-standard and would have been ignored by servers.
Fix formatting on HTTP Header for RSS Accceptable Content-Types introduced in commit 6896392
Some feeds are Content-Type application/xml only and will respond with a 406 error to responses with a header of content type application/rss+xml. This change allows for the more permissive content types application/xml and text/xml to be accepted.
Also fixes "UserAgent" header key with correct "User-Agent".