diff --git a/README.md b/README.md index f74bb09..ccb8397 100644 --- a/README.md +++ b/README.md @@ -146,6 +146,34 @@ signalReady() - `scrubSensitiveData(event)` - Sentry `beforeSend` hook that redacts values of settings keys matching `token`, `secret`, `password`, or `credential` with `[REDACTED]`. Drops the event if it cannot be safely serialized. - `reportError(error, context?)` - Capture an exception via Sentry with optional extra context. +### HTTP Requests + +- `fetchJson(url, options?)` - Fetch a URL and parse the response body as JSON. Throws a `FetchJsonError` (with `status`, `statusText`, `url`, and an optional `body` holding the parsed error payload when the server returned one) if the response is not ok. Throws a `FetchJsonParseError` (with `url` and the raw `body` string) if the response is ok but its body is not valid JSON. An ok response with an empty body resolves to `undefined`. +- `fetchJsonOrDefault(url, fallback, options?, warningMessage?)` - Same as `fetchJson`, but catches any error, logs it with `console.warn` (using `warningMessage` if provided), and returns `fallback` instead of throwing. + +Both requests are aborted after a default timeout of 8 seconds (`DEFAULT_TIMEOUT_MS`) unless overridden via `options.timeoutMs`. Pass `0` or `Infinity` to disable the timeout entirely. + +```typescript +const settings = await fetchJson('https://api.example.com/settings') + +const settingsWithFallback = await fetchJsonOrDefault( + 'https://api.example.com/settings', + defaultSettings, +) +``` + +```typescript +const settingsWithTimeout = await fetchJson( + 'https://api.example.com/settings', + { timeoutMs: 3000 }, +) + +const settingsNoTimeout = await fetchJson( + 'https://api.example.com/settings', + { timeoutMs: 0 }, +) +``` + ## Web Components This library includes reusable web components for building consistent Edge Apps. See the [components documentation](https://github.com/Screenly/edge-apps-library/blob/main/docs/components.md) for usage details. diff --git a/package-lock.json b/package-lock.json index 034b5cf..d3314c5 100644 --- a/package-lock.json +++ b/package-lock.json @@ -1,12 +1,12 @@ { "name": "@screenly/edge-apps", - "version": "1.2.1", + "version": "1.3.0", "lockfileVersion": 3, "requires": true, "packages": { "": { "name": "@screenly/edge-apps", - "version": "1.2.1", + "version": "1.3.0", "license": "MIT", "dependencies": { "@eslint/js": "^10.0.1", diff --git a/package.json b/package.json index ed38bdd..7b6797b 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "@screenly/edge-apps", - "version": "1.2.1", + "version": "1.3.0", "description": "A TypeScript library for interfacing with Screenly Edge Apps API", "type": "module", "sideEffects": [ diff --git a/src/utils/http.test.ts b/src/utils/http.test.ts new file mode 100644 index 0000000..8dc6189 --- /dev/null +++ b/src/utils/http.test.ts @@ -0,0 +1,374 @@ +import { describe, test, expect, afterEach, vi } from 'vitest' +import { + fetchJson, + fetchJsonOrDefault, + FetchJsonError, + FetchJsonParseError, + DEFAULT_TIMEOUT_MS, +} from './http' + +// eslint-disable-next-line max-lines-per-function +describe('http utilities', () => { + afterEach(() => { + vi.unstubAllGlobals() + vi.restoreAllMocks() + }) + + // eslint-disable-next-line max-lines-per-function + describe('fetchJson', () => { + test('should resolve with parsed JSON on a successful response', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response(JSON.stringify({ hello: 'world' }), { + status: 200, + headers: { 'Content-Type': 'application/json' }, + }) + }), + ) + + const data = await fetchJson<{ hello: string }>( + 'https://example.com/data', + ) + expect(data).toEqual({ hello: 'world' }) + }) + + test('should throw a FetchJsonError when the response is not ok', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response('Not Found', { + status: 404, + statusText: 'Not Found', + }) + }), + ) + + await expect(fetchJson('https://example.com/missing')).rejects.toThrow( + FetchJsonError, + ) + }) + + test('should include status, statusText, and url on a thrown FetchJsonError', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response('Server Error', { + status: 500, + statusText: 'Internal Server Error', + }) + }), + ) + + try { + await fetchJson('https://example.com/broken') + expect.unreachable('fetchJson should have thrown') + } catch (error) { + expect(error).toBeInstanceOf(FetchJsonError) + const fetchError = error as FetchJsonError + expect(fetchError.status).toBe(500) + expect(fetchError.statusText).toBe('Internal Server Error') + expect(fetchError.url).toBe('https://example.com/broken') + } + }) + + test('should attach the parsed JSON body to a thrown FetchJsonError', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response(JSON.stringify({ message: 'invalid api key' }), { + status: 401, + }) + }), + ) + + try { + await fetchJson('https://example.com/unauthorized') + expect.unreachable('fetchJson should have thrown') + } catch (error) { + expect(error).toBeInstanceOf(FetchJsonError) + const fetchError = error as FetchJsonError + expect(fetchError.body).toEqual({ message: 'invalid api key' }) + } + }) + + test('should leave body undefined on a FetchJsonError when the response is not valid JSON', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response('Not Found', { status: 404 }) + }), + ) + + try { + await fetchJson('https://example.com/missing') + expect.unreachable('fetchJson should have thrown') + } catch (error) { + expect(error).toBeInstanceOf(FetchJsonError) + const fetchError = error as FetchJsonError + expect(fetchError.body).toBeUndefined() + } + }) + + test('should throw a FetchJsonParseError when an ok response is not valid JSON', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response('not json', { status: 200 }) + }), + ) + + await expect(fetchJson('https://example.com/html')).rejects.toThrow( + FetchJsonParseError, + ) + }) + + test('should resolve with undefined when an ok response has an empty body', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response(null, { status: 204 }) + }), + ) + + const data = await fetchJson('https://example.com/no-content') + expect(data).toBeUndefined() + }) + + test('should propagate network errors from fetch', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.reject(new Error('Network error'))), + ) + + await expect(fetchJson('https://example.com/data')).rejects.toThrow( + 'Network error', + ) + }) + + test('should abort the request once timeoutMs elapses', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((_url: string, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(new DOMException('Aborted', 'AbortError')) + }) + }) + }), + ) + + await expect( + fetchJson('https://example.com/slow', { timeoutMs: 10 }), + ).rejects.toThrow('Aborted') + }) + + test('should apply the default timeout when timeoutMs is omitted', async () => { + vi.useFakeTimers() + vi.stubGlobal( + 'fetch', + vi.fn((_url: string, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(new DOMException('Aborted', 'AbortError')) + }) + }) + }), + ) + + const result = expect( + fetchJson('https://example.com/slow'), + ).rejects.toThrow('Aborted') + + await vi.advanceTimersByTimeAsync(DEFAULT_TIMEOUT_MS) + await result + + vi.useRealTimers() + }) + + test('should not abort by the default timeout when an explicit timeoutMs overrides it', async () => { + vi.useFakeTimers() + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response(JSON.stringify({ hello: 'world' }), { + status: 200, + }) + }), + ) + + const result = expect( + fetchJson('https://example.com/data', { + timeoutMs: DEFAULT_TIMEOUT_MS * 2, + }), + ).resolves.toEqual({ hello: 'world' }) + + await vi.advanceTimersByTimeAsync(DEFAULT_TIMEOUT_MS) + await result + + vi.useRealTimers() + }) + + test('should abort the request when a caller-provided signal aborts, even before timeoutMs elapses', async () => { + vi.stubGlobal( + 'fetch', + vi.fn((_url: string, init?: RequestInit) => { + return new Promise((_resolve, reject) => { + init?.signal?.addEventListener('abort', () => { + reject(new DOMException('Aborted', 'AbortError')) + }) + }) + }), + ) + + const callerController = new AbortController() + const result = expect( + fetchJson('https://example.com/slow', { + signal: callerController.signal, + timeoutMs: DEFAULT_TIMEOUT_MS * 2, + }), + ).rejects.toThrow('Aborted') + + callerController.abort() + await result + }) + }) + + // eslint-disable-next-line max-lines-per-function + describe('fetchJsonOrDefault', () => { + test('should resolve with parsed JSON on a successful response', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response(JSON.stringify({ hello: 'world' }), { + status: 200, + }) + }), + ) + + const data = await fetchJsonOrDefault<{ hello: string } | null>( + 'https://example.com/data', + null, + ) + expect(data).toEqual({ hello: 'world' }) + }) + + test('should return the fallback and log a warning on a non-ok response', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response('Not Found', { status: 404 }) + }), + ) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const data = await fetchJsonOrDefault('https://example.com/missing', []) + expect(data).toEqual([]) + expect(warnSpy).toHaveBeenCalledTimes(1) + }) + + test('should return the fallback and log a warning when the response is not valid JSON', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => { + return new Response('not json', { status: 200 }) + }), + ) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const data = await fetchJsonOrDefault('https://example.com/html', []) + expect(data).toEqual([]) + expect(warnSpy).toHaveBeenCalledWith( + 'Failed to fetch JSON:', + expect.any(FetchJsonParseError), + ) + }) + + test('should return the fallback on a network error', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(() => Promise.reject(new Error('Network error'))), + ) + vi.spyOn(console, 'warn').mockImplementation(() => {}) + + const data = await fetchJsonOrDefault('https://example.com/data', null) + expect(data).toBeNull() + }) + + test('should return fallback instead of undefined when an ok response has an empty body', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(null, { status: 204 })), + ) + + const arrayFallback = await fetchJsonOrDefault( + 'https://example.com/no-content', + [1, 2, 3], + ) + expect(arrayFallback).toEqual([1, 2, 3]) + + const objectFallback = await fetchJsonOrDefault<{ hello: string }>( + 'https://example.com/no-content', + { hello: 'fallback' }, + ) + expect(objectFallback).toEqual({ hello: 'fallback' }) + }) + + test('should use a custom warning message when provided', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('Not Found', { status: 404 })), + ) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + await fetchJsonOrDefault( + 'https://example.com/missing', + null, + {}, + 'Custom failure message:', + ) + + expect(warnSpy).toHaveBeenCalledWith( + 'Custom failure message:', + expect.any(FetchJsonError), + ) + }) + + test('should redact sensitive query params from the URL before logging a FetchJsonError', async () => { + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response('Not Found', { status: 404 })), + ) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + await fetchJsonOrDefault( + 'https://example.com/missing?appid=super-secret-key', + null, + ) + + expect(warnSpy).toHaveBeenCalledTimes(1) + const loggedError = warnSpy.mock.calls[0]?.[1] as FetchJsonError + expect(loggedError).toBeInstanceOf(FetchJsonError) + expect(loggedError.url).not.toContain('super-secret-key') + expect(loggedError.url).toContain('appid=%5BREDACTED%5D') + expect(loggedError.message).not.toContain('super-secret-key') + }) + + test('should truncate a long body when logging a FetchJsonParseError', async () => { + const longBody = 'x'.repeat(500) + vi.stubGlobal( + 'fetch', + vi.fn(async () => new Response(longBody, { status: 200 })), + ) + const warnSpy = vi.spyOn(console, 'warn').mockImplementation(() => {}) + + await fetchJsonOrDefault('https://example.com/html', null) + + expect(warnSpy).toHaveBeenCalledTimes(1) + const loggedError = warnSpy.mock.calls[0]?.[1] as FetchJsonParseError + expect(loggedError).toBeInstanceOf(FetchJsonParseError) + expect(loggedError.body.length).toBeLessThan(longBody.length) + expect(loggedError.body.endsWith('...')).toBe(true) + }) + }) +}) diff --git a/src/utils/http.ts b/src/utils/http.ts new file mode 100644 index 0000000..9eb6ec2 --- /dev/null +++ b/src/utils/http.ts @@ -0,0 +1,258 @@ +/** + * HTTP Utilities + * Shared helpers for making fetch requests and parsing JSON responses, + * reducing boilerplate around checking `response.ok` and error handling. + */ +import { SENSITIVE_KEY_PATTERNS } from './sentry.js' + +/** + * Query param name patterns treated as sensitive when redacting a URL for + * logging. Extends the shared `SENSITIVE_KEY_PATTERNS` from `sentry.ts` + * with a couple of patterns specific to URL query params (e.g. `appid`, + * `apikey`) without altering `sentry.ts`'s own behaviour. + */ +const URL_SENSITIVE_KEY_PATTERNS = [...SENSITIVE_KEY_PATTERNS, 'key', 'appid'] + +function isSensitiveQueryKey(key: string): boolean { + const lowerKey = key.toLowerCase() + return URL_SENSITIVE_KEY_PATTERNS.some((pattern) => + lowerKey.includes(pattern), + ) +} + +/** + * Redact the values of any sensitive-looking query params (e.g. `token`, + * `apikey`, `appid`) from a URL, for safe use in logs. Returns the original + * string unchanged if it cannot be parsed as a URL. + */ +export function redactUrl(url: string): string { + try { + const parsed = new URL(url) + for (const key of [...parsed.searchParams.keys()]) { + if (isSensitiveQueryKey(key)) { + parsed.searchParams.set(key, '[REDACTED]') + } + } + return parsed.toString() + } catch { + return url + } +} + +const MAX_LOGGED_BODY_LENGTH = 200 + +/** + * Truncate a response body for safe use in logs, to avoid dumping large or + * sensitive payloads into log output. + */ +export function truncateForLogging(body: string): string { + return body.length > MAX_LOGGED_BODY_LENGTH + ? `${body.slice(0, MAX_LOGGED_BODY_LENGTH)}...` + : body +} + +/** + * Options accepted by `fetchJson` and `fetchJsonOrDefault`. + * Extends the standard `fetch` options with an optional request timeout. + */ +export interface FetchJsonOptions extends RequestInit { + /** + * Abort the request after this many milliseconds. Defaults to + * `DEFAULT_TIMEOUT_MS` when omitted. Pass `0` or `Infinity` to disable + * the timeout entirely. If a `signal` is also provided, either it or the + * timeout aborting will cancel the request. + */ + timeoutMs?: number +} + +/** + * Default request timeout applied by `fetchJson` when the caller does not + * pass `timeoutMs`. Kept below the screenshotter's ~10s network-idle + * budget so a fallback/error path still has time to run. + */ +export const DEFAULT_TIMEOUT_MS = 8000 + +/** + * Error thrown by `fetchJson` when a response is received but its status + * is outside the 200-299 range. If the response body could be parsed as + * JSON, the parsed value is available as `body` so callers can surface + * API-provided error details (e.g. `{ "message": "invalid api key" }`). + * `body` is left `undefined` when the response body was empty or was not + * valid JSON, such as an HTML error page. + */ +export class FetchJsonError extends Error { + constructor( + message: string, + public readonly status: number, + public readonly statusText: string, + public readonly url: string, + public readonly body?: unknown, + ) { + super(message) + this.name = 'FetchJsonError' + } +} + +/** + * Error thrown by `fetchJson` when a response is ok (status 200-299) but + * its body is not valid JSON, such as an HTML error page served with a + * 200, or a plain-text body. Distinct from `FetchJsonError` so callers can + * tell "the request failed" apart from "the request succeeded but the + * body could not be parsed" without inspecting a native `SyntaxError`. + */ +export class FetchJsonParseError extends Error { + constructor( + message: string, + public readonly url: string, + public readonly body: string, + ) { + super(message) + this.name = 'FetchJsonParseError' + } +} + +/** + * Fetch a URL and parse the response body as JSON. + * + * The response body is read once via `response.text()` (a `Response` body + * can only be consumed once, so `.text()` and `.json()` cannot both be + * called on it), then parsed with `JSON.parse`. + * + * Throws a `FetchJsonError` if the response status is not ok (its `body` + * field carries the parsed error payload when the server returned one), a + * `FetchJsonParseError` if the response is ok but its body is not valid + * JSON, or the underlying `fetch` error (including an abort error on + * timeout) otherwise. An ok response with an empty body resolves to + * `undefined` rather than throwing, to accommodate responses such as a 204 + * No Content, so the resolved type is `T | undefined` and callers that + * require a value should check for `undefined` explicitly. Callers that + * want a non-throwing variant should use `fetchJsonOrDefault`. + * + * @param url - URL to request + * @param options - Standard fetch options, plus an optional `timeoutMs` to + * abort the request after a given duration (defaults to + * `DEFAULT_TIMEOUT_MS`; pass `0` or `Infinity` to disable) + */ +export async function fetchJson( + url: string, + options: FetchJsonOptions = {}, +): Promise { + const { + timeoutMs = DEFAULT_TIMEOUT_MS, + signal: callerSignal, + ...init + } = options + let timeoutId: ReturnType | undefined + let signal = callerSignal + + if (Number.isFinite(timeoutMs) && timeoutMs > 0) { + const controller = new AbortController() + timeoutId = setTimeout(() => controller.abort(), timeoutMs) + signal = callerSignal + ? AbortSignal.any([callerSignal, controller.signal]) + : controller.signal + } + + try { + const response = await fetch(url, { ...init, signal }) + const text = await response.text() + + if (!response.ok) { + let body: unknown + try { + body = JSON.parse(text) as unknown + } catch { + body = undefined + } + + throw new FetchJsonError( + `Request to ${url} failed with status ${response.status}`, + response.status, + response.statusText, + url, + body, + ) + } + + if (text === '') { + return undefined + } + + try { + return JSON.parse(text) as T + } catch { + throw new FetchJsonParseError( + `Response from ${url} was not valid JSON`, + url, + text, + ) + } + } finally { + if (timeoutId !== undefined) clearTimeout(timeoutId) + } +} + +/** + * Fetch a URL and parse the response body as JSON, returning `fallback` + * instead of throwing when the request fails (network error, timeout, a + * non-ok response status, or an ok response whose body is not valid + * JSON). Logs a `console.warn` with the failure reason. + * + * An ok response with an empty body also resolves to `fallback` here + * rather than `undefined`, since this function's contract is to always + * return a `T`. + * + * Useful for optional data where a failed request should not interrupt + * rendering, e.g. an Edge App falling back to a default value. + * + * @param url - URL to request + * @param fallback - Value returned when the request fails, or when an ok + * response resolves to `undefined` (an empty body) + * @param options - Standard fetch options, plus an optional `timeoutMs` + * @param warningMessage - Message logged (via `console.warn`) before the + * error, to give context on which request failed + */ +export async function fetchJsonOrDefault( + url: string, + fallback: T, + options: FetchJsonOptions = {}, + warningMessage = 'Failed to fetch JSON:', +): Promise { + try { + const result = await fetchJson(url, options) + return result === undefined ? fallback : result + } catch (error) { + console.warn(warningMessage, toLoggableError(error)) + return fallback + } +} + +/** + * Build a safe-to-log representation of an error thrown by `fetchJson`, + * with the URL's sensitive query params redacted and, for + * `FetchJsonParseError`, the raw body truncated. Used only for logging; + * callers still receive the original, unredacted error. + */ +function toLoggableError(error: unknown): unknown { + if (error instanceof FetchJsonParseError) { + const redactedUrl = redactUrl(error.url) + return new FetchJsonParseError( + `Response from ${redactedUrl} was not valid JSON`, + redactedUrl, + truncateForLogging(error.body), + ) + } + + if (error instanceof FetchJsonError) { + const redactedUrl = redactUrl(error.url) + return new FetchJsonError( + `Request to ${redactedUrl} failed with status ${error.status}`, + error.status, + error.statusText, + redactedUrl, + error.body, + ) + } + + return error +} diff --git a/src/utils/index.ts b/src/utils/index.ts index 9533328..f47c182 100644 --- a/src/utils/index.ts +++ b/src/utils/index.ts @@ -1,6 +1,7 @@ export * from './calendar.js' export * from './error-handling.js' export * from './html.js' +export * from './http.js' export * from './theme.js' export * from './locale.js' export * from './metadata.js' diff --git a/src/utils/oauth.ts b/src/utils/oauth.ts index ac0eafa..3770074 100644 --- a/src/utils/oauth.ts +++ b/src/utils/oauth.ts @@ -1,3 +1,5 @@ +import { fetchJson } from './http.js' + const TOKEN_REFRESH_INTERVAL_SEC = 30 * 60 /** @@ -33,21 +35,25 @@ export const initTokenRefreshLoop = (onRefresh: () => Promise): void => { * Retrieves credentials from the Screenly OAuth service * @param tokenType The token endpoint type (default: 'access_token') * @returns An object containing the token and optional metadata from the OAuth provider + * @throws {FetchJsonError} If the OAuth service responds with a non-ok status + * @throws {Error} If the OAuth service responds with an ok but empty body */ export const getCredentials = async ( tokenType: string = 'access_token', ): Promise<{ token: string; metadata?: Record }> => { - const response = await fetch( - screenly.settings.screenly_oauth_tokens_url + tokenType + '/', - { - method: 'GET', - headers: { - Accept: 'application/json', - Authorization: `Bearer ${screenly.settings.screenly_app_auth_token}`, - }, + const result = await fetchJson<{ + token: string + metadata?: Record + }>(screenly.settings.screenly_oauth_tokens_url + tokenType + '/', { + headers: { + Accept: 'application/json', + Authorization: `Bearer ${screenly.settings.screenly_app_auth_token}`, }, - ) + }) + + if (result === undefined) { + throw new Error('OAuth token endpoint returned an empty response') + } - const { token, metadata } = await response.json() - return { token, metadata } + return result } diff --git a/src/utils/sentry.ts b/src/utils/sentry.ts index c7acdb4..4a75ed5 100644 --- a/src/utils/sentry.ts +++ b/src/utils/sentry.ts @@ -3,9 +3,14 @@ import type { ErrorEvent } from '@sentry/browser' import { getHostname } from './metadata.js' import { getSetting, getSettings } from './settings.js' -const SENSITIVE_KEY_PATTERNS = ['token', 'secret', 'password', 'credential'] +export const SENSITIVE_KEY_PATTERNS = [ + 'token', + 'secret', + 'password', + 'credential', +] -function isSensitiveKey(key: string): boolean { +export function isSensitiveKey(key: string): boolean { const lowerKey = key.toLowerCase() return SENSITIVE_KEY_PATTERNS.some((pattern) => lowerKey.includes(pattern)) } diff --git a/src/utils/weather.ts b/src/utils/weather.ts index 16dd5c0..09b23c3 100644 --- a/src/utils/weather.ts +++ b/src/utils/weather.ts @@ -5,6 +5,7 @@ import { getSetting, type MeasurementUnit } from './settings.js' import { getMetadata } from './metadata.js' +import { fetchJsonOrDefault } from './http.js' // Import weather icons import clearIcon from '../assets/images/icons/clear.svg' @@ -185,6 +186,20 @@ export function isValidWeatherResponse(data: { ) } +/** + * Shape of the OpenWeatherMap "current weather" API response, limited to + * the fields actually read by `fetchCurrentWeatherData`. + */ +interface OpenWeatherMapCurrentResponse { + cod?: number | string + main?: { + temp?: number + temp_max?: number + temp_min?: number + } + weather?: Array<{ id?: number; description?: string }> +} + export interface CurrentWeatherRawData { temperature: number tempHigh: number @@ -216,35 +231,26 @@ export async function fetchCurrentWeatherData( return null } - const response = await fetch( + const data = await fetchJsonOrDefault( `https://api.openweathermap.org/data/2.5/weather?lat=${lat}&lon=${lng}&units=${unit}&appid=${apiKey}`, + null, + {}, + 'Failed to get weather data:', ) - if (!response.ok) { - console.warn( - 'Failed to get weather data: OpenWeatherMap API responded with', - response.status, - response.statusText, - ) + if (!data || !isValidWeatherResponse(data)) { return null } - const data = await response.json() - - if (!isValidWeatherResponse(data)) { - return null - } - - const temperature = Math.round(data.main.temp) + const main = data.main! + const temperature = Math.round(main.temp!) const tempHigh = - typeof data.main.temp_max === 'number' && - Number.isFinite(data.main.temp_max) - ? Math.round(data.main.temp_max) + typeof main.temp_max === 'number' && Number.isFinite(main.temp_max) + ? Math.round(main.temp_max) : temperature const tempLow = - typeof data.main.temp_min === 'number' && - Number.isFinite(data.main.temp_min) - ? Math.round(data.main.temp_min) + typeof main.temp_min === 'number' && Number.isFinite(main.temp_min) + ? Math.round(main.temp_min) : temperature const weatherId = data.weather?.[0]?.id ?? null @@ -281,6 +287,15 @@ export interface CityInfo { countryCode: string } +/** + * Shape of a single entry in the OpenWeatherMap reverse geocoding API + * response, limited to the fields actually read by `getCityInfo`. + */ +interface OpenWeatherMapGeoResult { + name?: string + country?: string +} + /** * Get city information including name and country code from OpenWeatherMap reverse geocoding * @param lat - Latitude @@ -288,48 +303,26 @@ export interface CityInfo { * @returns Object containing cityName and countryCode */ export async function getCityInfo(lat: number, lng: number): Promise { - try { - const apiKey = getSetting('openweathermap_api_key') - if (!apiKey) { - // Fallback to location from metadata if no API key - return { - cityName: getMetadata().location || 'Unknown Location', - countryCode: '', - } - } + const apiKey = getSetting('openweathermap_api_key') - const response = await fetch( + if (apiKey) { + const results = await fetchJsonOrDefault( `https://api.openweathermap.org/geo/1.0/reverse?lat=${lat}&lon=${lng}&limit=1&appid=${apiKey}`, + [], + {}, + 'Failed to get city info:', ) - if (!response.ok) { - console.warn( - 'Failed to get city info: OpenWeatherMap API responded with', - response.status, - response.statusText, - ) + const { name, country } = results[0] ?? {} + if (name && country) { return { - cityName: getMetadata().location || 'Unknown Location', - countryCode: '', - } - } - - const data = await response.json() - - if (Array.isArray(data) && data.length > 0) { - const { name, country } = data[0] - if (name && country) { - return { - cityName: `${name}, ${country}`, - countryCode: country, - } + cityName: `${name}, ${country}`, + countryCode: country, } } - } catch (error) { - console.warn('Failed to get city info:', error) } - // Fallback to location from metadata + // Fallback to location from metadata if no API key, no results, or the request failed return { cityName: getMetadata().location || 'Unknown Location', countryCode: '',