diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index d9e001913..b8e3dfc15 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -49,6 +49,16 @@ jobs: test_e2e: runs-on: ubuntu-latest + # This job needs secrets.GCP_SA_KEY to auth to GCP and pull private Docker + # images. GitHub does not pass repository secrets to `pull_request` + # workflow runs triggered from a forked repository (a platform-level + # security restriction, not something we control), so this job would + # otherwise fail immediately on every external contributor's PR before any + # test code runs. Skip it in that case rather than reporting a failure + # that has nothing to do with the PR's actual changes. It still runs + # normally for internal branches, pull requests within this repo, and + # pushes to master, where secrets are available. + if: github.event_name == 'push' || github.event.pull_request.head.repo.full_name == github.repository steps: - uses: actions/checkout@v4 - uses: google-github-actions/auth@v2 diff --git a/app/utils/location.ts b/app/utils/location.ts index 7766c0355..4914361ef 100644 --- a/app/utils/location.ts +++ b/app/utils/location.ts @@ -24,6 +24,17 @@ export const areCoordsInSanFrancisco = (coords: GeoCoordinates): boolean => { ); }; +/** + * Thrown by getLocationBrowser when the browser successfully returns a real + * location, but it falls outside the bounds we support. This is distinct + * from other rejection reasons (permission denied, unsupported browser, + * timeout, etc.) because it means we already have a real answer -- there's + * no reason to believe Google's (IP-based, less precise) Geolocation API + * would give a meaningfully different result, so callers can skip that + * billed API call and fall straight back to the default location. + */ +export class OutOfBoundsLocationError extends Error {} + /** * Get location via HTML5 Geolocation API. */ @@ -41,7 +52,7 @@ export const getLocationBrowser = () => } else { const msg = `User location out of bounds: ${coords.lat},${coords.lng}`; console.log(msg); // eslint-disable-line no-console - reject(msg); + reject(new OutOfBoundsLocationError(msg)); } }, (error) => { @@ -134,11 +145,22 @@ export const useDefaultSanFranciscoLocation = () => * inaccurate geolocation results, but this should be removed if more locations * are added. * - * @todo if getLocationBrowser is outside SF, errs and tries to load google as well. Fix * @returns A Promise of a location, which is either an object with `lat` and * `lng` properties or an error if location is unavaible or out of bounds. */ export const getLocation = () => - getLocationBrowser() - .catch(() => getLocationGoogle()) - .catch(() => useDefaultSanFranciscoLocation()); + getLocationBrowser().catch((reason) => { + if (reason instanceof OutOfBoundsLocationError) { + // The browser gave us a real, precise location -- it's just outside + // the area we support. Calling Google's (IP-based, less precise) + // Geolocation API here is very unlikely to give a meaningfully + // different answer, so skip that billed call and go straight to the + // default location. + return useDefaultSanFranciscoLocation(); + } + + // Any other rejection reason (permission denied, unsupported browser, + // timeout, etc.) means we don't have a real location yet, so it's + // still worth trying Google's Geolocation API as a fallback. + return getLocationGoogle().catch(() => useDefaultSanFranciscoLocation()); + });