Browse Source

made rss fetch more stable

imwald
Silberengel 4 months ago
parent
commit
481a08e3a2
  1. 44
      src/components/RssFeedList/index.tsx
  2. 75
      src/services/rss-feed.service.ts

44
src/components/RssFeedList/index.tsx

@ -15,7 +15,18 @@ export default function RssFeedList() {
const [error, setError] = useState<string | null>(null) const [error, setError] = useState<string | null>(null)
useEffect(() => { useEffect(() => {
// Create AbortController for this effect
const abortController = new AbortController()
let isMounted = true
let isLoading = false
const loadRssFeeds = async () => { const loadRssFeeds = async () => {
// Check if already aborted or if a load is already in progress
if (abortController.signal.aborted || isLoading) {
return
}
isLoading = true
setLoading(true) setLoading(true)
setError(null) setError(null)
@ -63,8 +74,18 @@ export default function RssFeedList() {
logger.info('[RssFeedList] No RSS feed list event in context, using default feeds') logger.info('[RssFeedList] No RSS feed list event in context, using default feeds')
} }
// Check if aborted before fetching
if (abortController.signal.aborted || !isMounted) {
return
}
// Fetch and merge feeds (this handles errors gracefully and returns partial results) // Fetch and merge feeds (this handles errors gracefully and returns partial results)
const fetchedItems = await rssFeedService.fetchMultipleFeeds(feedUrls) const fetchedItems = await rssFeedService.fetchMultipleFeeds(feedUrls, abortController.signal)
// Check if aborted after fetching
if (abortController.signal.aborted || !isMounted) {
return
}
if (fetchedItems.length === 0) { if (fetchedItems.length === 0) {
// No items were successfully fetched, but don't show error if we tried // No items were successfully fetched, but don't show error if we tried
@ -74,6 +95,16 @@ export default function RssFeedList() {
setItems(fetchedItems) setItems(fetchedItems)
} catch (err) { } catch (err) {
// Don't handle abort errors - they're expected during cleanup
if (err instanceof DOMException && err.name === 'AbortError') {
return
}
// Check if still mounted before setting error
if (!isMounted) {
return
}
logger.error('[RssFeedList] Error loading RSS feeds', { error: err }) logger.error('[RssFeedList] Error loading RSS feeds', { error: err })
// Don't set error state - fetchMultipleFeeds handles individual feed failures gracefully // Don't set error state - fetchMultipleFeeds handles individual feed failures gracefully
// Only set error if there's a critical issue (like network completely down) // Only set error if there's a critical issue (like network completely down)
@ -84,9 +115,13 @@ export default function RssFeedList() {
setError(err instanceof Error ? err.message : t('Failed to load RSS feeds')) setError(err instanceof Error ? err.message : t('Failed to load RSS feeds'))
} }
} finally { } finally {
isLoading = false
// Only update loading state if still mounted
if (isMounted) {
setLoading(false) setLoading(false)
} }
} }
}
loadRssFeeds() loadRssFeeds()
@ -94,7 +129,7 @@ export default function RssFeedList() {
const handleRssFeedListUpdate = (event: CustomEvent) => { const handleRssFeedListUpdate = (event: CustomEvent) => {
const detail = event.detail as { pubkey: string; feedUrls: string[]; eventId: string } const detail = event.detail as { pubkey: string; feedUrls: string[]; eventId: string }
// Only refresh if it's for the current user // Only refresh if it's for the current user
if (detail.pubkey === pubkey) { if (detail.pubkey === pubkey && isMounted) {
logger.info('[RssFeedList] Received RSS feed list update event, refreshing...', { logger.info('[RssFeedList] Received RSS feed list update event, refreshing...', {
eventId: detail.eventId, eventId: detail.eventId,
feedCount: detail.feedUrls.length feedCount: detail.feedUrls.length
@ -106,9 +141,12 @@ export default function RssFeedList() {
window.addEventListener('rssFeedListUpdated', handleRssFeedListUpdate as EventListener) window.addEventListener('rssFeedListUpdated', handleRssFeedListUpdate as EventListener)
return () => { return () => {
isMounted = false
isLoading = false
abortController.abort() // Cancel all in-flight requests
window.removeEventListener('rssFeedListUpdated', handleRssFeedListUpdate as EventListener) window.removeEventListener('rssFeedListUpdated', handleRssFeedListUpdate as EventListener)
} }
}, [pubkey, t]) }, [pubkey, rssFeedListEvent, t])
if (loading) { if (loading) {
return ( return (

75
src/services/rss-feed.service.ts

@ -66,19 +66,29 @@ class RssFeedService {
/** /**
* Fetch and parse an RSS/Atom feed from a URL * Fetch and parse an RSS/Atom feed from a URL
*/ */
async fetchFeed(url: string): Promise<RssFeed> { async fetchFeed(url: string, signal?: AbortSignal): Promise<RssFeed> {
// Check cache first // Check cache first
const cached = this.feedCache.get(url) const cached = this.feedCache.get(url)
if (cached && Date.now() - cached.timestamp < this.CACHE_DURATION) { if (cached && Date.now() - cached.timestamp < this.CACHE_DURATION) {
return cached.feed return cached.feed
} }
// Check if already aborted
if (signal?.aborted) {
throw new DOMException('The operation was aborted.', 'AbortError')
}
// Try multiple fetch strategies in order // Try multiple fetch strategies in order
const strategies = this.getFetchStrategies(url) const strategies = this.getFetchStrategies(url)
for (const strategy of strategies) { for (const strategy of strategies) {
// Check if aborted before trying next strategy
if (signal?.aborted) {
throw new DOMException('The operation was aborted.', 'AbortError')
}
try { try {
const xmlText = await this.fetchWithStrategy(url, strategy) const xmlText = await this.fetchWithStrategy(url, strategy, signal)
if (xmlText) { if (xmlText) {
const feed = this.parseFeed(xmlText, url) const feed = this.parseFeed(xmlText, url)
// Cache the feed // Cache the feed
@ -86,6 +96,10 @@ class RssFeedService {
return feed return feed
} }
} catch (error) { } catch (error) {
// Don't log abort errors as warnings - they're expected during cleanup
if (error instanceof DOMException && error.name === 'AbortError') {
throw error // Re-throw abort errors immediately
}
logger.warn('[RssFeedService] Strategy failed', { url, strategy: strategy.name, error }) logger.warn('[RssFeedService] Strategy failed', { url, strategy: strategy.name, error })
// Continue to next strategy // Continue to next strategy
continue continue
@ -135,11 +149,26 @@ class RssFeedService {
/** /**
* Fetch feed using a specific strategy * Fetch feed using a specific strategy
*/ */
private async fetchWithStrategy(originalUrl: string, strategy: { name: string; getUrl: (url: string) => string }): Promise<string> { private async fetchWithStrategy(originalUrl: string, strategy: { name: string; getUrl: (url: string) => string }, externalSignal?: AbortSignal): Promise<string> {
const fetchUrl = strategy.getUrl(originalUrl) const fetchUrl = strategy.getUrl(originalUrl)
// Check if external signal is already aborted
if (externalSignal?.aborted) {
throw new DOMException('The operation was aborted.', 'AbortError')
}
const controller = new AbortController() const controller = new AbortController()
const timeoutId = setTimeout(() => controller.abort(), 15000) // 15 second timeout const timeoutId = setTimeout(() => {
controller.abort()
}, 15000) // 15 second timeout
// If external signal is provided, abort our controller when external signal aborts
if (externalSignal) {
externalSignal.addEventListener('abort', () => {
clearTimeout(timeoutId)
controller.abort()
}, { once: true })
}
try { try {
const res = await fetch(fetchUrl, { const res = await fetch(fetchUrl, {
@ -172,6 +201,10 @@ class RssFeedService {
return xmlText return xmlText
} catch (error) { } catch (error) {
clearTimeout(timeoutId) clearTimeout(timeoutId)
// Re-throw abort errors as-is
if (error instanceof DOMException && error.name === 'AbortError') {
throw error
}
throw error throw error
} }
} }
@ -894,18 +927,29 @@ class RssFeedService {
* Fetch multiple feeds and merge items * Fetch multiple feeds and merge items
* This method gracefully handles failures - if some feeds fail, it returns items from successful feeds * This method gracefully handles failures - if some feeds fail, it returns items from successful feeds
*/ */
async fetchMultipleFeeds(feedUrls: string[]): Promise<RssFeedItem[]> { async fetchMultipleFeeds(feedUrls: string[], signal?: AbortSignal): Promise<RssFeedItem[]> {
if (feedUrls.length === 0) { if (feedUrls.length === 0) {
return [] return []
} }
// Check if already aborted
if (signal?.aborted) {
throw new DOMException('The operation was aborted.', 'AbortError')
}
const results = await Promise.allSettled( const results = await Promise.allSettled(
feedUrls.map(url => this.fetchFeed(url)) feedUrls.map(url => this.fetchFeed(url, signal))
) )
// Check if aborted after fetching
if (signal?.aborted) {
throw new DOMException('The operation was aborted.', 'AbortError')
}
const allItems: RssFeedItem[] = [] const allItems: RssFeedItem[] = []
let successCount = 0 let successCount = 0
let failureCount = 0 let failureCount = 0
let abortCount = 0
results.forEach((result, index) => { results.forEach((result, index) => {
if (result.status === 'fulfilled') { if (result.status === 'fulfilled') {
@ -914,8 +958,15 @@ class RssFeedService {
logger.debug('[RssFeedService] Successfully fetched feed', { url: feedUrls[index], itemCount: result.value.items.length }) logger.debug('[RssFeedService] Successfully fetched feed', { url: feedUrls[index], itemCount: result.value.items.length })
} else { } else {
failureCount++ failureCount++
const error = result.reason
// Don't log abort errors - they're expected during cleanup
if (error instanceof DOMException && error.name === 'AbortError') {
abortCount++
// Silently skip aborted requests
return
}
// Log warning but don't throw - we want to return partial results // Log warning but don't throw - we want to return partial results
const errorMessage = result.reason instanceof Error ? result.reason.message : String(result.reason) const errorMessage = error instanceof Error ? error.message : String(error)
logger.warn('[RssFeedService] Failed to fetch feed after trying all strategies', { logger.warn('[RssFeedService] Failed to fetch feed after trying all strategies', {
url: feedUrls[index], url: feedUrls[index],
error: errorMessage error: errorMessage
@ -923,20 +974,24 @@ class RssFeedService {
} }
}) })
// Log summary // Log summary (only if not aborted)
if (!signal?.aborted) {
if (successCount > 0) { if (successCount > 0) {
logger.info('[RssFeedService] Feed fetch summary', { logger.info('[RssFeedService] Feed fetch summary', {
total: feedUrls.length, total: feedUrls.length,
successful: successCount, successful: successCount,
failed: failureCount, failed: failureCount - abortCount, // Don't count aborts as failures
aborted: abortCount,
itemsFound: allItems.length itemsFound: allItems.length
}) })
} else if (failureCount > 0) { } else if (failureCount > abortCount) {
// Only log error if there were actual failures (not just aborts)
logger.error('[RssFeedService] All feeds failed to fetch', { logger.error('[RssFeedService] All feeds failed to fetch', {
total: feedUrls.length, total: feedUrls.length,
urls: feedUrls urls: feedUrls
}) })
} }
}
// Sort by publication date (newest first) // Sort by publication date (newest first)
allItems.sort((a, b) => { allItems.sort((a, b) => {

Loading…
Cancel
Save