feat: add shared fetchJson helper for API calls - #62
Open
nicomiguelino wants to merge 9 commits into
Open
Conversation
- Add fetchJson and fetchJsonOrDefault helpers in src/utils/http.ts that wrap fetch, check response.ok, and support an optional timeoutMs - fetchJson throws a FetchJsonError on a non-ok response, fetchJsonOrDefault catches failures, logs a warning, and returns a caller-supplied fallback - Refactor oauth.ts getCredentials, and weather.ts fetchCurrentWeatherData and getCityInfo to use the new helpers, removing duplicated fetch boilerplate - Add tests for the new http utilities
- Add DEFAULT_TIMEOUT_MS (8000ms) applied when timeoutMs is omitted - Keep the timeout overridable, including disabling it with 0 or Infinity - Add tests covering the default timeout and overriding it
- add HTTP Requests subsection under Core Functions - cover FetchJsonError, fallback behavior, and the 8s default timeout with timeoutMs override examples
- attach parsed error payload to FetchJsonError as an optional body field - add FetchJsonParseError for ok responses whose body is not valid JSON - resolve ok responses with an empty body to undefined instead of throwing - add test coverage for both new failure paths and fetchJsonOrDefault - document the new behavior in the README HTTP Requests section
- Bump package.json version from 1.2.1 to 1.3.0 - Sync package-lock.json version to match
- fetchJsonOrDefault no longer forwards undefined from an ok, empty-body response when a non-undefined fallback is provided - update JSDoc to document the empty-body fallback behavior - add test covering array and object fallbacks on a 204 response - restore console.warn spies via a shared afterEach instead of scattered per-test mockRestore calls, so a spy cannot leak into later tests
- rename repeated 'const settings' declarations in both HTTP Requests code blocks so each block is valid TypeScript on its own
…ined typing - combine caller-provided AbortSignal with the timeout controller's signal via AbortSignal.any so either one cancels the request - redact sensitive query params and truncate long bodies before console.warn logs a FetchJsonError or FetchJsonParseError in fetchJsonOrDefault, reusing sentry.ts's SENSITIVE_KEY_PATTERNS - change fetchJson's return type to Promise<T | undefined> to reflect that an empty response body resolves to undefined at runtime - update getCredentials in oauth.ts to throw when the OAuth token endpoint returns an empty body, keeping its own return type non-optional
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
fetchJsonandfetchJsonOrDefaulthelpers insrc/utils/http.tsto remove duplicatedfetchboilerplate (checkingresponse.ok, logging warnings, parsing JSON) across the library.fetchJsonthrows aFetchJsonErroron a non-ok response, letting callers catch and handle it however they need.fetchJsonOrDefaultwraps it, logs a warning, and returns a caller-supplied fallback instead of throwing.timeoutMsof 8 seconds (DEFAULT_TIMEOUT_MS) so requests are bounded by default, keeping the app inside the screenshotter's ~10s network-idle budget. Callers can override this by passing their owntimeoutMs, including0orInfinityto disable the timeout entirely.oauth.tsgetCredentials: now checksresponse.ok(previously it did not), throwingFetchJsonErroron failure.weather.tsfetchCurrentWeatherDataandgetCityInfo: swapped rawfetch+ manual checks forfetchJsonOrDefault, which also removed a duplicated fallback branch ingetCityInfo.theme.tsfetchLogoImagewas left untouched. It works withresponse.blob()/arrayBuffer()rather than JSON, so the new JSON-oriented helper is not a good fit there.Test plan
npm run type-checknpm run lintnpm run testnpm run format:checksrc/utils/http.test.tscovering success, non-ok response, network error, timeout, fallback behavior, the default timeout, and overriding the default timeout.