Claude/fix waze api integration wf1 x8#6
Open
mcherinx wants to merge 18 commits intoR44VC0RP:mainfrom
Open
Conversation
Replace the Waze georss endpoint (which is being blocked) with OpenWeb Ninja's official Waze API. This provides: - Reliable, documented API with proper authentication - Support for both alerts and traffic jams - Better rate limiting and error handling - Future-proof integration Changes: - New lib/openweb-ninja.ts: Utilities for bounds conversion and response mapping - Updated app/api/waze/route.ts: Switch to OpenWeb Ninja API, add API key validation - Updated types/waze.ts: Document provider field The API response format remains unchanged for backward compatibility with the frontend (useWazeAlerts hook). https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Security improvements: - Validate bounds are valid numbers and within lat/lng ranges - Ensure left < right and bottom < top for coordinate sanity - Use Authorization header instead of query parameter for API key (prevents logging/caching of credentials) - Sanitize error messages to avoid leaking upstream API details - Prevent NaN values from reaching external API calls This addresses: - Credential exposure via URL query parameters - Missing input validation - Information leakage in error messages - Coordinate validation edge cases https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Critical improvements: ✓ Per-IP rate limiting (15 req/min per IP + 60 req/min global) ✓ Request timeout protection (15 second timeout) ✓ Response size limits (5 MB max) ✓ Content-Type validation (require application/json) ✓ Request size limits (2 KB max query string) ✓ Cache key injection prevention (use SHA256 hash) ✓ Fail-secure rate limiting (reject on Redis error) ✓ Timeout handling with AbortController ✓ Sanitized logging (remove sensitive bounds) ✓ IP address extraction from multiple headers Security architecture: - Per-IP limit checked first (prevents distributed attacks) - Global limit checked second (fallback protection) - Request timeout prevents indefinite hangs - Response validation prevents memory exhaustion - Stale cache fallback for graceful degradation Error handling: - Distinguish timeout errors from API errors - Don't expose upstream error details - Fail securely on infrastructure failures https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
…cated Waze georss
…environment variables
…tch actual API return
Logs full error response body to help debug authentication issues. https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
- Add HTTPS requirement check (except localhost) - Provide specific error messages for each error code - Improve error display with actionable instructions - Help users understand why geolocation isn't working https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Remove Google Fonts import that was causing build failures due to network issues. Fall back to system fonts instead. https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
- Add 'Request Permission' button when location access is denied - Allow users to retry geolocation permission without browser restart - Improve error messages to guide users through permission process - Track permissionDenied state separately https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Add missing permissionDenied property to setState call https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Fix TypeScript errors in useGeolocation hook https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Add requestPermission callback to geolocation hook usage and show a 'Try Again' button when location error occurs on main page https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
Contributor
|
@claude is attempting to deploy a commit to the exon Team on Vercel. A member of the Team first needs to authorize it. |
📝 WalkthroughWalkthroughMigrates Waze upstream to OpenWeb Ninja with bounds conversion, response mapping, SHA‑256 cache keys, Redis caching and stale‑fallback. Adds request hardening: API key check, query length limit, per‑IP (15/min) and global (60/min) rate limits with PostHog logging, 15s fetch timeout, content validation, and expanded upstream error handling. Adds geolocation permission retry and removes Geist font import; middleware export changed. Changes
Sequence Diagram(s)sequenceDiagram
actor Client
participant Route as API Route<br/>(app/api/waze/route.ts)
participant Redis as Redis Cache
participant OpenWeb as OpenWeb Ninja<br/>API
participant PostHog as PostHog<br/>(Analytics)
Client->>Route: GET /api/waze?left&right&top&bottom[&jams=true]
Note over Route: Validate env/API key\nEnforce max query length
Route->>Route: Compute SHA-256 cache key (bounds[+jams])
Route->>Redis: GET cache
Redis-->>Route: cached response or null
alt Cache Hit
Route-->>Client: 200 + cached alerts
else Cache Miss
Route->>Route: Rate limit checks\n(per-IP 15/min, global 60/min)
alt Rate Limited
Route->>PostHog: Log rate limit event
Route-->>Client: 429 or 503
else Within Limits
Route->>OpenWeb: Fetch (Bearer token, 15s timeout)
alt Upstream Non‑OK
OpenWeb-->>Route: 401/403/429/other
Route->>PostHog: Log auth/error event (when applicable)
Route->>Redis: GET stale cache
Redis-->>Route: stale or null
Route-->>Client: stale cached data or 500
else Success
OpenWeb-->>Route: JSON response
Note over Route: Validate content-type & content-length\nMap OpenWeb→Waze format
Route->>Redis: SET cache
Route-->>Client: 200 + alerts
end
end
end
sequenceDiagram
actor User
participant UI as App Page<br/>(app/page.tsx / app/record/page.tsx)
participant Hook as useGeolocation<br/>(hooks/useGeolocation.ts)
participant Browser as Browser<br/>Geolocation API
UI->>Hook: initialize useGeolocation()
Note over Hook: Check secure context (HTTPS/localhost)
alt Not secure
Hook-->>UI: loading=false, geoError
UI->>User: show HTTPS error
else Secure
Hook->>Browser: getCurrentPosition() / watchPosition()
Browser->>User: permission prompt
alt Permission Denied
Browser-->>Hook: PositionError(code=1)
Hook->>Hook: set permissionDenied=true
Hook-->>UI: geoError + permissionDenied
UI->>User: show "Try Again" button
User->>UI: click "Try Again"
UI->>Hook: requestPermission()
Hook->>Browser: getCurrentPosition() (re-request)
else Permission Granted
Browser-->>Hook: Position
Hook->>Hook: update coords, heading, speed
Hook-->>UI: latitude, longitude, etc.
UI->>User: show location data
end
end
Estimated Code Review Effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
Prevent attempting geolocation requests on non-secure contexts https://claude.ai/code/session_016bxWrQjx3zbHHcCspGvm2m
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (5)
middleware.ts (1)
4-11: Avoid duplicating shutdown-flag parsing logic.Line 4-Line 11 re-implements logic already present in
lib/shutdown.ts. Consider reusing the shared parser/constant to prevent future drift between layout and middleware behavior.♻️ Suggested consolidation
+import { PROJECT_SHUTDOWN_ENABLED } from "@/lib/shutdown"; import { NextResponse } from "next/server"; import type { NextRequest } from "next/server"; - -function isShutdownEnabled(): boolean { - const shutdownValue = process.env.NEXT_PUBLIC_PROJECT_SHUTDOWN; - if (!shutdownValue) { - return true; - } - const disabledValues = new Set(["0", "false", "off", "no"]); - return !disabledValues.has(shutdownValue.trim().toLowerCase()); -} export default function middleware(request: NextRequest): NextResponse { - if (!isShutdownEnabled()) { + if (!PROJECT_SHUTDOWN_ENABLED) { return NextResponse.next(); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@middleware.ts` around lines 4 - 11, The isShutdownEnabled function duplicates shutdown-flag parsing already implemented in lib/shutdown.ts; replace the inline logic by importing and reusing the shared parser/constant from lib/shutdown.ts (e.g., the exported function or constant that computes shutdownEnabled/shutdownFlag) instead of re-creating disabledValues and parsing process.env.NEXT_PUBLIC_PROJECT_SHUTDOWN locally, updating any calls in middleware.ts to call that shared API (keep the isShutdownEnabled identifier calling the imported helper if you want to preserve usage).types/waze.ts (1)
17-17: Consider using a union type for stricter type safety.The comment documents the expected values but doesn't enforce them. A union type would provide compile-time safety.
Suggested improvement
- provider?: string; // "waze" | "openweb_ninja" + provider?: "waze" | "openweb_ninja";🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@types/waze.ts` at line 17, The provider field in the Waze type is currently typed as string with a comment; change it to a string literal union to enforce allowed values (e.g., provider?: "waze" | "openweb_ninja") so the TypeScript compiler catches invalid assignments—update the provider property in the type/interface declaration in types/waze.ts accordingly.lib/openweb-ninja.ts (1)
101-101: Using||treats0as falsy, potentially skipping valid zero reliability.If
alert.reliabilityis explicitly0, this falls through toconfidence. Use nullish coalescing (??) if0should be preserved as a valid value.Suggested fix
- reliability: alert.reliability || alert.confidence || 0, + reliability: alert.reliability ?? alert.confidence ?? 0,🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@lib/openweb-ninja.ts` at line 101, The assignment for the output property reliability currently uses logical OR which treats 0 as falsy; change the expression in the reliability assignment (where you set reliability: alert.reliability || alert.confidence || 0) to use nullish coalescing so zero is preserved, i.e. replace the OR chain with alert.reliability ?? alert.confidence ?? 0; locate this in the code that builds the object from alert (the reliability property assignment in lib/openweb-ninja.ts).app/api/waze/route.ts (1)
142-148: Consider deferring PostHog shutdown to avoid blocking responses.Each
await posthog.shutdown()blocks the response while flushing events. In serverless environments this may be necessary, but consider usingwaitUntil(if available in your runtime) to flush after the response is sent.// If using Vercel/Next.js edge runtime with waitUntil: // context.waitUntil(posthog.shutdown());This pattern is repeated at lines 166-172, 317-323, 346-352, and 411-419.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@app/api/waze/route.ts` around lines 142 - 148, The current code calls await posthog.shutdown() immediately after capturing events (see getPostHogClient() and the posthog.capture() calls), which blocks the response; change these to defer shutdown so telemetry flushes after response: if your runtime exposes context.waitUntil use context.waitUntil(posthog.shutdown()), otherwise invoke posthog.shutdown() without awaiting (or schedule it on a background task) to avoid blocking; update all occurrences around the posthog.capture() blocks (the instances following getPostHogClient() at the shown locations).hooks/useGeolocation.ts (1)
202-213: Replace manual secure context checks withwindow.isSecureContextin both locations.Lines 203 and 255 duplicate the same manual protocol/hostname check. Use
window.isSecureContextinstead—it's the standard way to gate geolocation and correctly handles localhost, 127.0.0.1, and IPv6 loopback (::1). This eliminates duplication and matches the browser's actual secure context rules.Current code (lines 203 and 255)
const isSecureContext = window.location.protocol === 'https:' || window.location.hostname === 'localhost' || window.location.hostname === '127.0.0.1';Apply to both the effect hook (line 203) and the
requestPermissioncallback (line 255).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useGeolocation.ts` around lines 202 - 213, Replace the manual secure-context logic with the browser API by using window.isSecureContext wherever the current manual check is used: in the effect hook that currently declares isSecureContext and in the requestPermission callback inside the useGeolocation hook; update the condition to check window.isSecureContext and remove the hostname/protocol comparisons so the early-return branch that calls setState({... error: "Geolocation requires HTTPS..."}) and returns behaves the same but uses the standard secure-context check.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@app/api/waze/route.ts`:
- Around line 338-358: In the 401 handling block in app/api/waze/route.ts (the
response.status === 401 branch), avoid consuming the response body twice; first
call response.text() to get the rawBody, then try JSON.parse(rawBody) (or use
JSON.try/catch) to extract structured errorBody for logging, falling back to
rawBody if parsing fails, and then proceed to call
getPostHogClient().capture(...) and shutdown; update the console.error calls to
use the parsed errorBody or rawBody so you don't call response.json() and
response.text() on the same Response object.
- Around line 56-61: Replace the separate redis.incr(key) followed by
redis.expire(key, TTL) with a single atomic transaction using redis.multi()/exec
to avoid the race; specifically, in the function that currently does "const
count = await redis.incr(key); if (count === 1) await redis.expire(key,
CACHE_TTL.RATE_LIMIT_WINDOW);" (and the analogous block in
checkGlobalRateLimit), call const results = await
redis.multi().incr(key).expire(key, CACHE_TTL.RATE_LIMIT_WINDOW).exec(), extract
the incremented count from the first result entry and use that instead of the
previous count variable, and handle a null/failed exec by treating it as an
error (or retry) so the key always gets an expiry set atomically.
In `@app/record/page.tsx`:
- Around line 101-111: The non-permission branch (where permissionDenied is
false) currently only shows static instructions; add an in-place retry control
so transient geolocation errors can be retried without navigation. Update the
JSX block that checks {!permissionDenied} to include an accessible "Retry"
button which invokes the existing geolocation request helper (e.g., call the
component's getLocation / requestLocation / fetchGeolocation function or create
a new handler named retryGeolocation / handleRetry that wraps the same logic),
and wire it to component state for loading/error feedback (e.g., disable the
button while retrying and show a spinner or status). Ensure the button text and
aria attributes are descriptive and reuse the permissionDenied boolean and
existing error state handling to keep behavior consistent.
In `@hooks/useGeolocation.ts`:
- Around line 242-268: requestPermission currently only calls
navigator.geolocation.getCurrentPosition, so after a manual retry the one-shot
succeeds but the continuous watcher started at the earlier watchPosition block
never restarts; update requestPermission to, after a successful
getCurrentPosition (i.e. where updatePosition is invoked), start the continuous
watcher the same way the watcher at the watchPosition block does — either call
the existing function that creates the watcher (e.g. startWatching / startWatch)
or invoke navigator.geolocation.watchPosition(updatePosition, handleError,
options) and save its id into the same watchId/state used by the watcher so
continuous updates resume.
In `@scripts/test-waze-bounds.ts`:
- Around line 62-66: The GET request created in the fetch call in
scripts/test-waze-bounds.ts unnecessarily sets a "Content-Type" header; remove
the "Content-Type" entry (or the entire headers object) from the fetch
invocation so the GET has no request body header — locate the fetch(url, {
headers: { "Content-Type": "application/json" } }) call and delete that header
(or the headers block) to fix it.
---
Nitpick comments:
In `@app/api/waze/route.ts`:
- Around line 142-148: The current code calls await posthog.shutdown()
immediately after capturing events (see getPostHogClient() and the
posthog.capture() calls), which blocks the response; change these to defer
shutdown so telemetry flushes after response: if your runtime exposes
context.waitUntil use context.waitUntil(posthog.shutdown()), otherwise invoke
posthog.shutdown() without awaiting (or schedule it on a background task) to
avoid blocking; update all occurrences around the posthog.capture() blocks (the
instances following getPostHogClient() at the shown locations).
In `@hooks/useGeolocation.ts`:
- Around line 202-213: Replace the manual secure-context logic with the browser
API by using window.isSecureContext wherever the current manual check is used:
in the effect hook that currently declares isSecureContext and in the
requestPermission callback inside the useGeolocation hook; update the condition
to check window.isSecureContext and remove the hostname/protocol comparisons so
the early-return branch that calls setState({... error: "Geolocation requires
HTTPS..."}) and returns behaves the same but uses the standard secure-context
check.
In `@lib/openweb-ninja.ts`:
- Line 101: The assignment for the output property reliability currently uses
logical OR which treats 0 as falsy; change the expression in the reliability
assignment (where you set reliability: alert.reliability || alert.confidence ||
0) to use nullish coalescing so zero is preserved, i.e. replace the OR chain
with alert.reliability ?? alert.confidence ?? 0; locate this in the code that
builds the object from alert (the reliability property assignment in
lib/openweb-ninja.ts).
In `@middleware.ts`:
- Around line 4-11: The isShutdownEnabled function duplicates shutdown-flag
parsing already implemented in lib/shutdown.ts; replace the inline logic by
importing and reusing the shared parser/constant from lib/shutdown.ts (e.g., the
exported function or constant that computes shutdownEnabled/shutdownFlag)
instead of re-creating disabledValues and parsing
process.env.NEXT_PUBLIC_PROJECT_SHUTDOWN locally, updating any calls in
middleware.ts to call that shared API (keep the isShutdownEnabled identifier
calling the imported helper if you want to preserve usage).
In `@types/waze.ts`:
- Line 17: The provider field in the Waze type is currently typed as string with
a comment; change it to a string literal union to enforce allowed values (e.g.,
provider?: "waze" | "openweb_ninja") so the TypeScript compiler catches invalid
assignments—update the provider property in the type/interface declaration in
types/waze.ts accordingly.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 5948d002-fd4d-4dec-ab42-c1d5a5244f7e
⛔ Files ignored due to path filters (1)
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing (9)
app/api/waze/route.tsapp/layout.tsxapp/page.tsxapp/record/page.tsxhooks/useGeolocation.tslib/openweb-ninja.tsmiddleware.tsscripts/test-waze-bounds.tstypes/waze.ts
Comment on lines
+56
to
+61
| try { | ||
| const count = await redis.incr(key); | ||
|
|
||
| if (count === 1) { | ||
| await redis.expire(key, CACHE_TTL.RATE_LIMIT_WINDOW); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
Upstash Redis pipeline atomic operations
💡 Result:
In Upstash Redis, pipelines and atomic operations are distinct features. Pipelines allow sending multiple Redis commands in a single HTTP request for reduced latency and round-trip time. They guarantee command ordering but are not atomic—commands from other clients can interleave, and partial failures are possible. Example in TypeScript SDK: const p = redis.pipeline; p.set("key", 2); p.incr("key"); const res = await p.exec; Auto-pipelining can be enabled for automatic batching. For atomicity, use transactions via multi / exec, which wrap commands in Redis MULTI/EXEC. These execute atomically with no interleaving. Example: const p = redis.multi; p.set("key", 2); p.incr("key"); const res = await p.exec; This matches standard Redis behavior, adapted for Upstash's serverless REST API. Use transactions for operations requiring isolation, pipelines for performance where atomicity is not needed.
Citations:
- 1: https://upstash.com/docs/redis/sdks/ts/pipelining/pipeline-transaction
- 2: https://upstash.com/docs/redis/sdks/ts/commands/transaction
- 3: https://upstash.com/blog/pipeline
- 4: https://upstash.com/docs/redis/sdks/py/features
🏁 Script executed:
# Check the full context of both rate limit functions mentioned
cd app/api/waze && wc -l route.tsRepository: R44VC0RP/teslanav.com
Length of output: 77
🏁 Script executed:
# Read the rate limiting functions (lines 51-99)
sed -n '51,99p' app/api/waze/route.tsRepository: R44VC0RP/teslanav.com
Length of output: 1358
🏁 Script executed:
# Also check the imports to understand which Redis client library is being used
head -20 app/api/waze/route.tsRepository: R44VC0RP/teslanav.com
Length of output: 926
Race condition between incr and expire could cause keys to persist indefinitely.
If the expire call fails after incr succeeds, the rate limit key may never expire, permanently blocking clients. Use atomic operations with multi/exec instead of separate calls.
Suggested fix using atomic transaction
try {
- const count = await redis.incr(key);
-
- if (count === 1) {
- await redis.expire(key, CACHE_TTL.RATE_LIMIT_WINDOW);
- }
+ // Use transaction for atomic operation
+ const multi = redis.multi();
+ multi.incr(key);
+ if (count === 1) {
+ multi.expire(key, CACHE_TTL.RATE_LIMIT_WINDOW);
+ }
+ const results = await multi.exec();
+ const count = results[0] as number;Note: The same issue exists in checkGlobalRateLimit at lines 82-96. Apply the same fix there.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/waze/route.ts` around lines 56 - 61, Replace the separate
redis.incr(key) followed by redis.expire(key, TTL) with a single atomic
transaction using redis.multi()/exec to avoid the race; specifically, in the
function that currently does "const count = await redis.incr(key); if (count ===
1) await redis.expire(key, CACHE_TTL.RATE_LIMIT_WINDOW);" (and the analogous
block in checkGlobalRateLimit), call const results = await
redis.multi().incr(key).expire(key, CACHE_TTL.RATE_LIMIT_WINDOW).exec(), extract
the incremented count from the first result entry and use that instead of the
previous count variable, and handle a null/failed exec by treating it as an
error (or retry) so the key always gets an expiry set atomically.
Comment on lines
+338
to
+358
| if (response.status === 401) { | ||
| try { | ||
| const errorBody = await response.json(); | ||
| console.error("OpenWeb Ninja API authentication failed (401):", errorBody); | ||
| } catch { | ||
| const errorText = await response.text(); | ||
| console.error("OpenWeb Ninja API authentication failed (401):", errorText); | ||
| } | ||
| const posthog = getPostHogClient(); | ||
| posthog.capture({ | ||
| distinctId: "server", | ||
| event: "openweb_ninja_auth_error", | ||
| properties: { status: 401 }, | ||
| }); | ||
| await posthog.shutdown(); | ||
|
|
||
| return NextResponse.json( | ||
| { error: "API configuration error", alerts: [] }, | ||
| { status: 500 } | ||
| ); | ||
| } |
There was a problem hiding this comment.
Response body consumed twice causes silent failure.
Once response.json() throws (e.g., for non-JSON error bodies), the body stream is exhausted. The subsequent response.text() will fail or return empty string, losing the error details.
Fix: read text first, then parse
// Handle API authentication errors
if (response.status === 401) {
+ const errorText = await response.text();
try {
- const errorBody = await response.json();
+ const errorBody = JSON.parse(errorText);
console.error("OpenWeb Ninja API authentication failed (401):", errorBody);
} catch {
- const errorText = await response.text();
console.error("OpenWeb Ninja API authentication failed (401):", errorText);
}
const posthog = getPostHogClient();📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if (response.status === 401) { | |
| try { | |
| const errorBody = await response.json(); | |
| console.error("OpenWeb Ninja API authentication failed (401):", errorBody); | |
| } catch { | |
| const errorText = await response.text(); | |
| console.error("OpenWeb Ninja API authentication failed (401):", errorText); | |
| } | |
| const posthog = getPostHogClient(); | |
| posthog.capture({ | |
| distinctId: "server", | |
| event: "openweb_ninja_auth_error", | |
| properties: { status: 401 }, | |
| }); | |
| await posthog.shutdown(); | |
| return NextResponse.json( | |
| { error: "API configuration error", alerts: [] }, | |
| { status: 500 } | |
| ); | |
| } | |
| if (response.status === 401) { | |
| const errorText = await response.text(); | |
| try { | |
| const errorBody = JSON.parse(errorText); | |
| console.error("OpenWeb Ninja API authentication failed (401):", errorBody); | |
| } catch { | |
| console.error("OpenWeb Ninja API authentication failed (401):", errorText); | |
| } | |
| const posthog = getPostHogClient(); | |
| posthog.capture({ | |
| distinctId: "server", | |
| event: "openweb_ninja_auth_error", | |
| properties: { status: 401 }, | |
| }); | |
| await posthog.shutdown(); | |
| return NextResponse.json( | |
| { error: "API configuration error", alerts: [] }, | |
| { status: 500 } | |
| ); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/api/waze/route.ts` around lines 338 - 358, In the 401 handling block in
app/api/waze/route.ts (the response.status === 401 branch), avoid consuming the
response body twice; first call response.text() to get the rawBody, then try
JSON.parse(rawBody) (or use JSON.try/catch) to extract structured errorBody for
logging, falling back to rawBody if parsing fails, and then proceed to call
getPostHogClient().capture(...) and shutdown; update the console.error calls to
use the parsed errorBody or rawBody so you don't call response.json() and
response.text() on the same Response object.
Comment on lines
+101
to
+111
| {!permissionDenied && ( | ||
| <div className="bg-blue-50 border border-blue-200 rounded-lg p-4 text-left text-sm text-blue-900"> | ||
| <p className="font-semibold mb-2">How to fix:</p> | ||
| <ul className="list-disc list-inside space-y-1"> | ||
| <li>Ensure you're using HTTPS</li> | ||
| <li>Check browser location permissions</li> | ||
| <li>Allow location access when prompted</li> | ||
| <li>Restart your browser and try again</li> | ||
| </ul> | ||
| </div> | ||
| )} |
There was a problem hiding this comment.
Add an in-place retry action for non-permission geolocation errors.
Timeout/unavailable errors are often transient, but this branch only shows instructions. Add a retry button here too, so users can recover without navigation.
💡 Suggested fix
{!permissionDenied && (
<div className="bg-blue-50 border border-blue-200 rounded-lg p-4 text-left text-sm text-blue-900">
<p className="font-semibold mb-2">How to fix:</p>
<ul className="list-disc list-inside space-y-1">
<li>Ensure you're using HTTPS</li>
<li>Check browser location permissions</li>
<li>Allow location access when prompted</li>
<li>Restart your browser and try again</li>
</ul>
+ <button
+ onClick={requestPermission}
+ className="mt-4 w-full px-4 py-3 bg-blue-500 hover:bg-blue-600 text-white font-semibold rounded-lg transition-colors"
+ >
+ Try Again
+ </button>
</div>
)}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| {!permissionDenied && ( | |
| <div className="bg-blue-50 border border-blue-200 rounded-lg p-4 text-left text-sm text-blue-900"> | |
| <p className="font-semibold mb-2">How to fix:</p> | |
| <ul className="list-disc list-inside space-y-1"> | |
| <li>Ensure you're using HTTPS</li> | |
| <li>Check browser location permissions</li> | |
| <li>Allow location access when prompted</li> | |
| <li>Restart your browser and try again</li> | |
| </ul> | |
| </div> | |
| )} | |
| {!permissionDenied && ( | |
| <div className="bg-blue-50 border border-blue-200 rounded-lg p-4 text-left text-sm text-blue-900"> | |
| <p className="font-semibold mb-2">How to fix:</p> | |
| <ul className="list-disc list-inside space-y-1"> | |
| <li>Ensure you're using HTTPS</li> | |
| <li>Check browser location permissions</li> | |
| <li>Allow location access when prompted</li> | |
| <li>Restart your browser and try again</li> | |
| </ul> | |
| <button | |
| onClick={requestPermission} | |
| className="mt-4 w-full px-4 py-3 bg-blue-500 hover:bg-blue-600 text-white font-semibold rounded-lg transition-colors" | |
| > | |
| Try Again | |
| </button> | |
| </div> | |
| )} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@app/record/page.tsx` around lines 101 - 111, The non-permission branch (where
permissionDenied is false) currently only shows static instructions; add an
in-place retry control so transient geolocation errors can be retried without
navigation. Update the JSX block that checks {!permissionDenied} to include an
accessible "Retry" button which invokes the existing geolocation request helper
(e.g., call the component's getLocation / requestLocation / fetchGeolocation
function or create a new handler named retryGeolocation / handleRetry that wraps
the same logic), and wire it to component state for loading/error feedback
(e.g., disable the button while retrying and show a spinner or status). Ensure
the button text and aria attributes are descriptive and reuse the
permissionDenied boolean and existing error state handling to keep behavior
consistent.
Comment on lines
+242
to
+268
| // Request permission manually (for when user taps "Request Permission") | ||
| const requestPermission = useCallback(() => { | ||
| if (!navigator.geolocation) { | ||
| setState((prev) => ({ | ||
| ...prev, | ||
| error: "Geolocation is not supported", | ||
| loading: false, | ||
| })); | ||
| return; | ||
| } | ||
|
|
||
| setState((prev) => ({ | ||
| ...prev, | ||
| loading: true, | ||
| error: null, | ||
| permissionDenied: false, | ||
| })); | ||
|
|
||
| const options: PositionOptions = { | ||
| enableHighAccuracy, | ||
| timeout: 10000, | ||
| maximumAge: 0, | ||
| }; | ||
|
|
||
| // Request position - this triggers the permission prompt | ||
| navigator.geolocation.getCurrentPosition(updatePosition, handleError, options); | ||
| }, [enableHighAccuracy, updatePosition, handleError]); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Verify current retry behavior is one-shot and where watchPosition is established.
rg -nP 'const requestPermission|getCurrentPosition|watchPosition|setWatchId' hooks/useGeolocation.ts -C3Repository: R44VC0RP/teslanav.com
Length of output: 1433
Re-enable continuous tracking after manual permission retry.
The requestPermission function at line 281 only calls getCurrentPosition, so recovery is one-shot. After the user denies permission initially, the watchPosition watcher (line 225) never starts. When the user later grants permission via requestPermission, continuous updates resume only for that single call—the watcher is not restarted, and location updates stop after the first success.
💡 Suggested fix
const requestPermission = useCallback(() => {
@@
// Request position - this triggers the permission prompt
- navigator.geolocation.getCurrentPosition(updatePosition, handleError, options);
- }, [enableHighAccuracy, updatePosition, handleError]);
+ navigator.geolocation.getCurrentPosition(
+ (position) => {
+ updatePosition(position);
+ // Ensure continuous updates resume after a successful retry
+ if (watchId === null) {
+ const id = navigator.geolocation.watchPosition(updatePosition, handleError, options);
+ setWatchId(id);
+ }
+ },
+ handleError,
+ options
+ );
+ }, [enableHighAccuracy, updatePosition, handleError, watchId]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@hooks/useGeolocation.ts` around lines 242 - 268, requestPermission currently
only calls navigator.geolocation.getCurrentPosition, so after a manual retry the
one-shot succeeds but the continuous watcher started at the earlier
watchPosition block never restarts; update requestPermission to, after a
successful getCurrentPosition (i.e. where updatePosition is invoked), start the
continuous watcher the same way the watcher at the watchPosition block does —
either call the existing function that creates the watcher (e.g. startWatching /
startWatch) or invoke navigator.geolocation.watchPosition(updatePosition,
handleError, options) and save its id into the same watchId/state used by the
watcher so continuous updates resume.
Comment on lines
62
to
66
| const response = await fetch(url, { | ||
| headers: { | ||
| "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36", | ||
| "Referer": "https://www.waze.com/live-map", | ||
| "Content-Type": "application/json", | ||
| }, | ||
| }); |
There was a problem hiding this comment.
Unnecessary Content-Type header on GET request.
Content-Type describes the body of a request, but GET requests typically have no body. This header can be removed.
Suggested fix
const response = await fetch(url, {
- headers: {
- "Content-Type": "application/json",
- },
+ headers: {},
});🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@scripts/test-waze-bounds.ts` around lines 62 - 66, The GET request created in
the fetch call in scripts/test-waze-bounds.ts unnecessarily sets a
"Content-Type" header; remove the "Content-Type" entry (or the entire headers
object) from the fetch invocation so the GET has no request body header — locate
the fetch(url, { headers: { "Content-Type": "application/json" } }) call and
delete that header (or the headers block) to fix it.
There was a problem hiding this comment.
♻️ Duplicate comments (1)
hooks/useGeolocation.ts (1)
242-283:⚠️ Potential issue | 🟠 MajorManual permission retry is still one-shot and doesn’t re-establish continuous updates.
After a successful manual retry,
requestPermissiononly callsgetCurrentPositionand does not ensure an activewatchPositionsubscription. This can leave tracking non-continuous after recovery flows.💡 Suggested fix
const requestPermission = useCallback(() => { @@ - // Request position - this triggers the permission prompt - navigator.geolocation.getCurrentPosition(updatePosition, handleError, options); + // Request position - this triggers the permission prompt + navigator.geolocation.getCurrentPosition( + (position) => { + updatePosition(position); + // Re-establish continuous tracking after successful retry + setWatchId((current) => { + if (current !== null) { + navigator.geolocation.clearWatch(current); + } + return navigator.geolocation.watchPosition(updatePosition, handleError, options); + }); + }, + handleError, + options + ); }, [enableHighAccuracy, updatePosition, handleError]);#!/bin/bash set -euo pipefail # Verify requestPermission currently performs one-shot fetch only. rg -nP 'const requestPermission|getCurrentPosition|watchPosition|setWatchId' hooks/useGeolocation.ts -C4 # Expected before fix: # - requestPermission block contains getCurrentPosition # - requestPermission block does NOT contain watchPosition/setWatchId restart logic🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useGeolocation.ts` around lines 242 - 283, requestPermission currently only calls navigator.geolocation.getCurrentPosition(updatePosition, handleError, options) which performs a one-shot fetch and doesn't re-establish continuous tracking; wrap or replace the success callback so that after a successful getCurrentPosition it also starts the watchPosition subscription (using the same PositionOptions) and stores the returned watch id via the existing setWatchId/state updater (or call the existing startWatching helper if one exists), and ensure you don't create duplicate watches if a watch is already active; reference requestPermission, updatePosition, handleError, watchPosition, setWatchId and the PositionOptions constant when making this change.
🧹 Nitpick comments (1)
hooks/useGeolocation.ts (1)
191-213: Extract shared geolocation preflight checks to avoid drift.The
navigator.geolocationand secure-context checks are duplicated in two places. A small shared helper would keep error text/state behavior consistent.Also applies to: 242-265
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@hooks/useGeolocation.ts` around lines 191 - 213, The geolocation support and secure-context preflight logic is duplicated in the hook (seen in the useEffect block checking navigator.geolocation and computing isSecureContext), so extract that into a single helper (e.g., function ensureGeolocationAvailable() or isGeolocationAllowed()) that returns a normalized result or throws/returns an error object; replace both duplicated blocks with calls to that helper and update callers to use the helper's standardized error message/state (setState, permissionDenied, loading) to keep behavior consistent across useEffect and the other location that currently repeats the checks.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@hooks/useGeolocation.ts`:
- Around line 242-283: requestPermission currently only calls
navigator.geolocation.getCurrentPosition(updatePosition, handleError, options)
which performs a one-shot fetch and doesn't re-establish continuous tracking;
wrap or replace the success callback so that after a successful
getCurrentPosition it also starts the watchPosition subscription (using the same
PositionOptions) and stores the returned watch id via the existing
setWatchId/state updater (or call the existing startWatching helper if one
exists), and ensure you don't create duplicate watches if a watch is already
active; reference requestPermission, updatePosition, handleError, watchPosition,
setWatchId and the PositionOptions constant when making this change.
---
Nitpick comments:
In `@hooks/useGeolocation.ts`:
- Around line 191-213: The geolocation support and secure-context preflight
logic is duplicated in the hook (seen in the useEffect block checking
navigator.geolocation and computing isSecureContext), so extract that into a
single helper (e.g., function ensureGeolocationAvailable() or
isGeolocationAllowed()) that returns a normalized result or throws/returns an
error object; replace both duplicated blocks with calls to that helper and
update callers to use the helper's standardized error message/state (setState,
permissionDenied, loading) to keep behavior consistent across useEffect and the
other location that currently repeats the checks.
![vercel[bot]](https://avatars.githubusercontent.com/in/8329?s=60&v=4){kind=small}
Comment on lines
+339
to
+343
| try { | ||
| const errorBody = await response.json(); | ||
| console.error("OpenWeb Ninja API authentication failed (401):", errorBody); | ||
| } catch { | ||
| const errorText = await response.text(); |
Contributor
There was a problem hiding this comment.
Suggested change
| try { | |
| const errorBody = await response.json(); | |
| console.error("OpenWeb Ninja API authentication failed (401):", errorBody); | |
| } catch { | |
| const errorText = await response.text(); | |
| // Read text first, then try to parse as JSON (body can only be read once) | |
| const errorText = await response.text(); | |
| try { | |
| const errorBody = JSON.parse(errorText); | |
| console.error("OpenWeb Ninja API authentication failed (401):", errorBody); | |
| } catch { |
Response body is consumed by response.json() making subsequent response.text() call fail in the catch block
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
2 participants
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 by CodeRabbit
New Features
Improvements