Skip to content

[core] refactor: extract and centralize HTTP fetcher#4016

Merged
khassel merged 3 commits intoMagicMirrorOrg:developfrom
KristjanESPERANTO:http-fetcher
Jan 22, 2026
Merged

[core] refactor: extract and centralize HTTP fetcher#4016
khassel merged 3 commits intoMagicMirrorOrg:developfrom
KristjanESPERANTO:http-fetcher

Conversation

@KristjanESPERANTO
Copy link
Collaborator

@KristjanESPERANTO KristjanESPERANTO commented Jan 20, 2026

Summary

PR #3976 introduced smart HTTP error handling for the Calendar module. This PR extracts that HTTP logic into a central HTTPFetcher class.

Calendar is the first module to use it. Follow-up PRs would migrate Newsfeed and maybe even Weather.

Before this change:

  • ❌ Each module had to implemented its own fetch() calls
  • ❌ No centralized retry logic or backoff strategies
  • ❌ No timeout handling for hanging requests
  • ❌ Error detection relied on fragile string parsing

What this PR adds:

  • ✅ Unified HTTPFetcher class with intelligent retry strategies
  • ✅ Modern AbortController with configurable timeout (default 30s)
  • ✅ Proper undici Agent for self-signed certificates
  • ✅ Structured error objects with translation keys
  • ✅ Calendar module migrated as first consumer
  • ✅ Comprehensive unit tests with msw (Mock Service Worker)

Architecture

Before - Decentralized HTTP handling:

Calendar Module          Newsfeed Module         Weather Module
┌─────────────┐         ┌─────────────┐         ┌─────────────┐
│ fetch() own │         │ fetch() own │         │ fetch() own │
│ retry logic │         │ basic error │         │ no retry    │
│ error parse │         │   handling  │         │ client-side │
└─────────────┘         └─────────────┘         └─────────────┘
      │                       │                       │
      └───────────────────────┴───────────────────────┘
                              ▼
                        External APIs

After - Centralized with HTTPFetcher:

┌─────────────────────────────────────────────────────┐
│                  HTTPFetcher                        │
│  • Unified retry strategies (401/403, 429, 5xx)     │
│  • AbortController timeout (30s)                    │
│  • Structured errors with translation keys          │
│  • undici Agent for self-signed certs               │
└────────────┬──────────────┬──────────────┬──────────┘
             │              │              │
     ┌───────▼───────┐ ┌────▼─────┐ ┌──────▼──────┐
     │   Calendar    │ │ Newsfeed │ │   Weather   │
     │   ✅ This PR  │ │  future  │ │   future    │
     └───────────────┘ └──────────┘ └─────────────┘
             │              │              │
             └──────────────┴──────────────┘
                          ▼
                   External APIs

Complexity Considerations

Does HTTPFetcher add complexity?

Even if it may look more complex, it actually reduces overall complexity:

Future Benefits

Weather migration (future PR):

Moving Weather from client-side to server-side will enable:

  • Same robust error handling - Weather gets 429 rate-limiting, 5xx backoff for free
  • Simpler architecture - No proxy layer needed

Moving the weather modules from client-side to server-side will be a big undertaking, but I think it's a good strategy. Even if we only move the calendar and newsfeed to the new HTTP fetcher and leave the weather as it is, this PR still makes sense, I think.

Breaking Changes

None


I am eager to hear your opinion on this 🙂

- Extract HTTP logic from Calendar module into HTTPFetcher class
- Migrate calendar module to use centralized HTTPFetcher
- Add modern improvements:
  * AbortController with configurable timeout (default 30s)
  * undici Agent for self-signed certificates
  * Improved Retry-After header handling (respects server hints)
- Implement intelligent retry strategies:
  * 401/403: 5x interval, min 30 minutes
  * 429: Retry-After header parsing, fallback 15 minutes
  * 5xx: Exponential backoff (max 3 retries)
  * 4xx: 2x interval, min 15 minutes
- Provide structured error objects with translation keys
- Comprehensive unit tests with msw (Mock Service Worker)
- Tests cover all error scenarios and authentication methods
Copy link
Collaborator

@rejas rejas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Awesome idea, I really like!

Just one thing: We dont add the default english translations to other languages. If a translation isnt available, it automatically takes the english one.

@KristjanESPERANTO
Copy link
Collaborator Author

My idea was to make it easier for the translators, but you're right, that was not very clean. Instead of taking the easy way out and simply removing them, I used automatic translation (which has delivered very good results in the past) to translate the strings. In doing so, I even noticed duplicate strings in the pt translation, which I removed right away.

@khassel khassel merged commit 34913bf into MagicMirrorOrg:develop Jan 22, 2026
9 checks passed
@KristjanESPERANTO KristjanESPERANTO deleted the http-fetcher branch January 22, 2026 19:56
@khassel
Copy link
Collaborator

khassel commented Jan 22, 2026

This broke my calendar:

[2026-01-22 23:27:21.769] [ERROR] [calendar] https://calendar.google.com/calendar/ical/xxx.calendar.google.com/public/basic.ics - iCal parsing failed: UNTIL rule part MUST have the same value type as DTSTART 

[2026-01-22 23:27:21.770] [ERROR] [calendar] Calendar Error. Could not fetch calendar:  https://calendar.google.com/calendar/ical/xxx.calendar.google.com/public/basic.ics iCal parsing failed: UNTIL rule part MUST have the same value type as DTSTART 

@khassel
Copy link
Collaborator

khassel commented Jan 22, 2026

Found the breaking entry:

DTSTART;VALUE=DATE:20160313
DTEND;VALUE=DATE:20160314
RRULE:FREQ=YEARLY;UNTIL=20190312;BYMONTHDAY=13;BYMONTH=3
DTSTAMP:20260122T223427Z
UID:a21aa3d4-c1c2-4d74-b929-535e168a566f
CREATED:20170127T230833Z
LAST-MODIFIED:20190330T190642Z
SEQUENCE:1
STATUS:CONFIRMED
SUMMARY:xxx
TRANSP:OPAQUE
X-MOZ-FAKED-MASTER:1
X-MOZ-GENERATION:2
END:VEVENT

I was able to solve the problem by deleting and recreating the entire calendar series in Google Calendar, but other users might encounter similar issues...

@KristjanESPERANTO
Copy link
Collaborator Author

Oh. I'll take a look at it by the weekend at the latest.

@khassel
Copy link
Collaborator

khassel commented Jan 22, 2026

don't know if it is a real problem, that was the only entry with a RRULE in my birthday calendar and the UNTIL=20190312 was in the past ...

@sdetweil
Copy link
Collaborator

I think it is confused between full day and not for some reason, rrule until doesn’t use DATE: as it’s not required there

KristjanESPERANTO added a commit to KristjanESPERANTO/node-ical that referenced this pull request Jan 23, 2026
When parsing recurring events with VALUE=DATE (all-day events),
preserve the VALUE=DATE parameter in the DTSTART string passed to
rrule-temporal. This ensures that rrule-temporal can correctly
validate that UNTIL values have the same type as DTSTART.

Previously, date-only information was lost when building the RRULE
string, causing rrule-temporal to incorrectly assume DTSTART was
DATE-TIME and reject valid DATE-only UNTIL values with the error:
"UNTIL rule part MUST have the same value type as DTSTART"

The fix uses local date components (getFullYear, getMonth, getDate)
which is consistent with how dateOnly dates are created in the
dateParameter function using new Date(year, month, day).

Fixes issue reported in MagicMirrorOrg/MagicMirror#4016 where
Google Calendar birthday events with yearly RRULE and DATE-only
UNTIL in the past failed to parse.

Test coverage:
- Yearly recurring DATE events with UNTIL
- Monthly recurring DATE events with UNTIL
- DATE events with COUNT instead of UNTIL
- Edge case: VALUE=DATE with TZID (correctly ignores TZID)
@KristjanESPERANTO
Copy link
Collaborator Author

I think I found the root cause 🙂

The error comes from rrule-temporal v1.4.2, which added yesterday stricter RFC 5545 validation. The library now correctly enforces that UNTIL must match DTSTART's type - if DTSTART is a DATE (all-day event), UNTIL must also be a DATE.

The problem: node-ical never told rrule-temporal that DTSTART was VALUE=DATE, so the validation failed. The stricter validation is good - it just exposed a bug in node-ical that's been there all along.

Recreating the entry helped because Google Calendar probably removed the UNTIL or changed the format. But the underlying issue remains for other users with similar entries.

I've opened a fix: jens-maus/node-ical#432

This should handle Google Calendar birthdays and similar recurring all-day events correctly.

KristjanESPERANTO added a commit to KristjanESPERANTO/node-ical that referenced this pull request Jan 24, 2026
When parsing recurring events with VALUE=DATE (all-day events),
preserve the VALUE=DATE parameter in the DTSTART string passed to
rrule-temporal. This ensures that rrule-temporal can correctly
validate that UNTIL values have the same type as DTSTART.

Previously, date-only information was lost when building the RRULE
string, causing rrule-temporal to incorrectly assume DTSTART was
DATE-TIME and reject valid DATE-only UNTIL values with the error:
"UNTIL rule part MUST have the same value type as DTSTART"

The fix uses local date components (getFullYear, getMonth, getDate)
which is consistent with how dateOnly dates are created in the
dateParameter function using new Date(year, month, day).

Fixes issue reported in MagicMirrorOrg/MagicMirror#4016 where
Google Calendar birthday events with yearly RRULE and DATE-only
UNTIL in the past failed to parse.

Test coverage:
- Yearly recurring DATE events with UNTIL
- Monthly recurring DATE events with UNTIL
- DATE events with COUNT instead of UNTIL
- Edge case: VALUE=DATE with TZID (correctly ignores TZID)
jens-maus pushed a commit to jens-maus/node-ical that referenced this pull request Jan 26, 2026
When parsing recurring events with VALUE=DATE (all-day events),
preserve the VALUE=DATE parameter in the DTSTART string passed to
rrule-temporal. This ensures that rrule-temporal can correctly
validate that UNTIL values have the same type as DTSTART.

Previously, date-only information was lost when building the RRULE
string, causing rrule-temporal to incorrectly assume DTSTART was
DATE-TIME and reject valid DATE-only UNTIL values with the error:
"UNTIL rule part MUST have the same value type as DTSTART"

The fix uses local date components (getFullYear, getMonth, getDate)
which is consistent with how dateOnly dates are created in the
dateParameter function using new Date(year, month, day).

Fixes issue reported in MagicMirrorOrg/MagicMirror#4016 where
Google Calendar birthday events with yearly RRULE and DATE-only
UNTIL in the past failed to parse.

Test coverage:
- Yearly recurring DATE events with UNTIL
- Monthly recurring DATE events with UNTIL
- DATE events with COUNT instead of UNTIL
- Edge case: VALUE=DATE with TZID (correctly ignores TZID)
khassel pushed a commit that referenced this pull request Jan 27, 2026
This includes a new version of `node-ical` which should resolve a
calendar issue that was reported
[here](#4016 (comment))
and
[here](#4010 (comment)).
KristjanESPERANTO added a commit to KristjanESPERANTO/MagicMirror that referenced this pull request Jan 28, 2026
Migrates the Newsfeed module to use the centralized HTTPFetcher class
introduced in MagicMirrorOrg#4016, following the same pattern as the Calendar module.

Changes:
- Refactored NewsfeedFetcher from function constructor to ES6 class
- Replaced manual fetch() + timer handling with HTTPFetcher composition
- Uses structured error objects with translation keys
- Inherits smart retry strategies (401/403, 429, 5xx backoff)
- Inherits timeout handling (30s) and AbortController
- Removed js/module_functions.js (scheduleTimer no longer needed)
- Removed #module_functions import from package.json
rejas pushed a commit that referenced this pull request Jan 29, 2026
This migrates the Newsfeed module to use the centralized HTTPFetcher
class (introduced in #4016), following the same pattern as the Calendar
module.

This continues the refactoring effort to centralize HTTP error handling
across all modules.

## Changes

**NewsfeedFetcher:**
- Refactored from function constructor to ES6 class (like the calendar
module in #3959)
- Replaced manual fetch() + timer handling with HTTPFetcher composition
- Uses structured error objects with translation keys
- Inherits smart retry strategies (401/403, 429, 5xx backoff)
- Inherits timeout handling (30s) and AbortController

**node_helper.js:**
- Updated error handler to use `errorInfo.translationKey`
- Simplified property access (`fetcher.url`, `fetcher.items`)

**Cleanup:**
- Removed `js/module_functions.js` (`scheduleTimer` no longer needed)
- Removed `#module_functions` import from package.json

## Related

Part of the HTTPFetcher migration effort started in #4016.
Next candidate: Weather module (client-side → server-side migration).
rejas pushed a commit that referenced this pull request Feb 23, 2026
… HTTPFetcher (#4032)

This migrates the Weather module from client-side fetching to use the
server-side centralized HTTPFetcher (introduced in #4016), following the
same pattern as the Calendar and Newsfeed modules.

## Motivation

This brings consistent error handling and better maintainability and
completes the refactoring effort to centralize HTTP error handling
across all default modules.

Migrating to server-side providers with HTTPFetcher brings:
- **Centralized error handling**: Inherits smart retry strategies
(401/403, 429, 5xx backoff) and timeout handling (30s)
- **Consistency**: Same architecture as Calendar and Newsfeed modules
- **Security**: Possibility to hide API keys/secrets from client-side
- **Performance**: Reduced API calls in multi-client setups - one server
fetch instead of one per client
- **Enabling possible future features**: e.g. server-side caching, rate
limit monitoring, and data sharing with third-party modules

## Changes

- All 10 weather providers now use HTTPFetcher for server-side fetching
- Consistent error handling like Calendar and Newsfeed modules

## Breaking Changes

None. Existing configurations continue to work.

## Testing

To ensure proper functionality, I obtained API keys and credentials for
all providers that require them. I configured all 10 providers in a
carousel setup and tested each one individually. Screenshots for each
provider are attached below demonstrating their working state.

I even requested developer access from the Tempest/WeatherFlow team to
properly test this provider.

**Comprehensive test coverage**: A major advantage of the server-side
architecture is the ability to thoroughly test providers with unit tests
using real API response snapshots. Don't be alarmed by the many lines
added in this PR - they are primarily test files and real-data mocks
that ensure provider reliability.

## Review Notes

I know this is an enormous change - I've been working on this for quite
some time. Unfortunately, breaking it into smaller incremental PRs
wasn't feasible due to the interdependencies between providers and the
shared architecture.

Given the scope, it's nearly impossible to manually review every change.
To ensure quality, I've used both CodeRabbit and GitHub Copilot to
review the code multiple times in my fork, and both provided extensive
and valuable feedback. Most importantly, my test setup with all 10
providers working successfully is very encouraging.

## Related

Part of the HTTPFetcher migration #4016.

## Screenshots

<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-06-54"
src="https://github.com/user-attachments/assets/2139f4d2-2a9b-4e49-8d0a-e4436983ed6e"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-02"
src="https://github.com/user-attachments/assets/880f7ce2-4e44-42d5-bfe4-5ce475cca7c2"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-07"
src="https://github.com/user-attachments/assets/abd89933-fe03-40ab-8a7c-41ae1ff99255"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-12"
src="https://github.com/user-attachments/assets/22225852-f0a9-4d33-87ab-0733ba30fad3"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-17"
src="https://github.com/user-attachments/assets/7a7192a5-f237-4060-85d7-6f50b9bef5af"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-22"
src="https://github.com/user-attachments/assets/df84d9f1-e531-4995-8da8-d6f2601b6a08"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-27"
src="https://github.com/user-attachments/assets/4cf391ac-db43-4b52-95f4-f5eadc5ea34d"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-32"
src="https://github.com/user-attachments/assets/8dd8e688-d47f-4815-87f6-7f2630f15d58"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-37"
src="https://github.com/user-attachments/assets/ee84a8bc-6b35-405a-b311-88658d9268dd"
/>
<img width="1920" height="1080" alt="Ekrankopio de 2026-02-08 13-07-42"
src="https://github.com/user-attachments/assets/f941f341-453f-4d4d-a8d9-6b9158eb2681"
/>

Provider "Weather API" added later:

<img width="1910" height="1080" alt="Ekrankopio de 2026-02-15 19-39-06"
src="https://github.com/user-attachments/assets/3f0c8ba3-105c-4f90-8b2e-3a1be543d3d2"
/>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants