fix: stabilize emulator location override fetch and logging#90
fix: stabilize emulator location override fetch and logging#90mattrossman wants to merge 1 commit intomainfrom
Conversation
|
✅ Preview build available. Download forecaswatch2-pbw-pr-90.zip Artifacts are attached to the workflow run/job and require GitHub access to this repository. |
There was a problem hiding this comment.
Pull request overview
This PR improves developer/emulator ergonomics around location override and boot-time fetching, while hardening some geocoding/reverse-geocoding paths to avoid PKJS crashes or confusing logs.
Changes:
- Harden reverse-geocode parsing and improve geocode logging readability.
- Tighten lat/long override detection with a stricter regex.
- Add
forceFetchOnBoothandling fordev-config.jsand document usage in the README.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
src/pkjs/weather/provider.js |
Adjusts reverse-geocode handling and location override parsing/logging. |
src/pkjs/index.js |
Adds forceFetchOnBoot dev-config behavior by clearing lastFetchSuccess. |
README.md |
Documents dev-config.js usage with location and forceFetchOnBoot example. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| var address = JSON.parse(response).address; | ||
| var body = JSON.parse(response); | ||
| if (!body.address) { | ||
| console.log('[!] Reverse geocoding response had no address object'); |
There was a problem hiding this comment.
If the reverse-geocode response has no address, this function returns without invoking callback, which will stall the whole fetch chain (fetch() waits on withCityName). Consider still calling callback with a safe fallback (e.g., empty/"Unknown") and/or propagating failure so the fetch can complete deterministically.
| console.log('[!] Reverse geocoding response had no address object'); | |
| console.log('[!] Reverse geocoding response had no address object'); | |
| callback('Unknown'); |
| var url = 'https://us1.locationiq.com/v1/search.php?key=' + locationiq_key | ||
| + '&q=' + this.location | ||
| + '&q=' + locationQuery | ||
| + '&format=json'; |
There was a problem hiding this comment.
locationQuery is interpolated into the LocationIQ URL without URL-encoding. Queries like the README example ("New York, NY") contain spaces/comma and should be passed through encodeURIComponent to avoid malformed requests or unexpected server parsing.
| if (locations.length === 0) { | ||
| console.log('[!] No geocoding results') | ||
| } |
There was a problem hiding this comment.
When the geocoding API returns an empty result set (locations.length === 0), the code only logs and never calls callback, which can also stall the fetch flow. Consider invoking the callback with a failure signal/fallback coordinates so downstream logic can proceed or fail cleanly.
|
@mattrossman I've opened a new pull request, #91, to work on those changes. Once the pull request is ready, I'll request review from you. |
forceFetchOnBootsupport insrc/pkjs/dev-config.jsprocessing so emulator boots can force an immediate weather fetch without opening settings.New York, NY) and improve geocode log output readability.ref: support lat/long in override location and avoid location lookup #59