3 Commits

Author SHA1 Message Date
Kristjan ESPERANTO
2e97e29ab5 fix(http_fetcher): use undici.fetch when dispatcher is present (#4097)
### What's the problem?

The `selfSignedCert` option passes an undici `Agent` as `dispatcher` to
`fetch()`. But Node's built-in `fetch()` and undici@8's `Agent` use
different internal handler APIs - passing them together throws:

```
invalid onRequestStart method
```

### What's the fix?

When `selfSignedCert` is enabled (i.e. a `dispatcher` is set), use
undici's own `fetch()` instead of the global one. For all other
requests, keep using `globalThis.fetch`.

```js
const fetchFn = requestOptions.dispatcher ? undiciFetch : globalThis.fetch;
```

### Why not just always use undici's fetch?

That would fix the crash - but it would break some tests. MSW (Mock
Service Worker), which is used in our test suite to intercept HTTP
requests, only hooks into `globalThis.fetch`. Undici's fetch bypasses
those interceptors entirely, so tests would start making real network
requests instead of getting the mocked responses. We could rewrite all
tests to use undici-compatible mocking instead - but that would be a
massive change for no real benefit.

----

Fixes #4093
2026-04-08 18:42:30 +02:00
Karsten Hassel
5e0cd28980 update electron to v40, update node versions in workflows (#4018)
- remove param `--enable-features=UseOzonePlatform` in start electron
tests (as we did already in `package.json`)
- update node versions in github workflows, remove `22.21.1`, add `25.x`
- fix formatting in tests
- update dependencies including electron to v40

This is still a draft PR because most calendar electron tests are not
running which is caused by the electron update from `v39.3.0` to
`v40.0.0`. Maybe @KristjanESPERANTO has an idea ...

---------

Co-authored-by: Kristjan ESPERANTO <35647502+KristjanESPERANTO@users.noreply.github.com>
2026-01-24 06:15:15 -06:00
Kristjan ESPERANTO
34913bfb9f [core] refactor: extract and centralize HTTP fetcher (#4016)
## Summary

PR [#3976](https://github.com/MagicMirrorOrg/MagicMirror/pull/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**:

- **Calendar already has this logic** (PR #3976) - we're extracting, not
adding
- **Alternative is worse:** Each module implementing own logic = 3× the
code
- **Better testability:** 443 lines of tests once vs. duplicating tests
for each module
- **Standards-based:** Retry-After is RFC 7231, not custom logic

## 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 🙂
2026-01-22 19:24:37 +01:00