Add fullscreen 3D Map page with geolocation + IP fallback#2
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
❌ Deploy Preview for pwawake failed.
|
There was a problem hiding this comment.
Summary
This PR adds a full-screen 3D map page with geolocation and IP fallback, improving the application's feature set. The implementation includes proper fallback logic and error handling in the frontend.
Critical Issues Found
Security Vulnerability: The API token for ipinfo.io is hardcoded in the source code, which exposes credentials and violates security best practices. This must be moved to environment variables before merging.
Missing Error Handling: The API handler lacks try-catch blocks to handle fetch failures, network errors, and JSON parsing errors, which will cause unhandled promise rejections and crash the handler.
Required Actions
- Move the ipinfo.io API token to an environment variable (e.g.,
process.env.IPINFO_TOKEN) - Wrap the API handler logic in a try-catch block to gracefully handle errors
- Add the environment variable to your deployment configuration
Once these security and error handling issues are resolved, the PR will be ready to merge.
You can now have the agent implement changes and create commits directly on your pull request's source branch. Simply comment with /q followed by your request in natural language to ask the agent to make changes.
| const ipResponse = await fetch(`https://ipinfo.io/${clientIP}/json?token=ef8461623ce04a`) | ||
| const clientIp = getClientIp(req) | ||
| const ipResponse = await fetch( | ||
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, |
There was a problem hiding this comment.
🛑 Security Vulnerability: Hardcoded API token exposes credentials in source code. Move the ipinfo.io API token to an environment variable to prevent unauthorized access and token theft.1
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | |
| ` |
Footnotes
-
CWE-798: Use of Hard-coded Credentials - https://cwe.mitre.org/data/definitions/798.html ↩
| const ipResponse = await fetch( | ||
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | ||
| ) | ||
| const apiResponse: ApiResponse = await ipResponse.json() |
There was a problem hiding this comment.
Add error handling for fetch failures. Network errors or API downtime will crash the handler without proper error handling.1
| const ipResponse = await fetch( | |
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | |
| ) | |
| const apiResponse: ApiResponse = await ipResponse.json() | |
| try { | |
| const ipResponse = await fetch( | |
| ` | |
| ) | |
| if (!ipResponse.ok) { | |
| res.status(200).json(fallbackLocation) | |
| return | |
| } | |
| const apiResponse: ApiResponse = await ipResponse.json() |
Footnotes
-
CWE-755: Improper Handling of Exceptional Conditions - https://cwe.mitre.org/data/definitions/755.html ↩
| export default async function handler( | ||
| req: NextApiRequest, | ||
| res: NextApiResponse<Data> | ||
| res: NextApiResponse<Data>, | ||
| ) { | ||
| const clientIP = req.headers['cf-connecting-ip'] | ||
| const ipResponse = await fetch(`https://ipinfo.io/${clientIP}/json?token=ef8461623ce04a`) | ||
| const clientIp = getClientIp(req) | ||
| const ipResponse = await fetch( | ||
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | ||
| ) | ||
| const apiResponse: ApiResponse = await ipResponse.json() | ||
|
|
||
| const [lat, long] = apiResponse.loc.split(",") | ||
| if (!apiResponse.loc) { | ||
| res.status(200).json(fallbackLocation) | ||
| return | ||
| } | ||
|
|
||
| const [lat, long] = apiResponse.loc.split(',') | ||
| const data: Data = { | ||
| latitude: parseFloat(lat), | ||
| longitude: parseFloat(long) | ||
| longitude: parseFloat(long), | ||
| } | ||
|
|
||
| res.status(200).json(data) |
There was a problem hiding this comment.
Add error handling for JSON parsing and API response errors. Without a try-catch block, fetch failures, network errors, and JSON parsing errors will cause unhandled promise rejections and crash the API handler.1
| export default async function handler( | |
| req: NextApiRequest, | |
| res: NextApiResponse<Data> | |
| res: NextApiResponse<Data>, | |
| ) { | |
| const clientIP = req.headers['cf-connecting-ip'] | |
| const ipResponse = await fetch(`https://ipinfo.io/${clientIP}/json?token=ef8461623ce04a`) | |
| const clientIp = getClientIp(req) | |
| const ipResponse = await fetch( | |
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | |
| ) | |
| const apiResponse: ApiResponse = await ipResponse.json() | |
| const [lat, long] = apiResponse.loc.split(",") | |
| if (!apiResponse.loc) { | |
| res.status(200).json(fallbackLocation) | |
| return | |
| } | |
| const [lat, long] = apiResponse.loc.split(',') | |
| const data: Data = { | |
| latitude: parseFloat(lat), | |
| longitude: parseFloat(long) | |
| longitude: parseFloat(long), | |
| } | |
| res.status(200).json(data) | |
| export default async function handler( | |
| req: NextApiRequest, | |
| res: NextApiResponse<Data>, | |
| ) { | |
| try { | |
| const clientIp = getClientIp(req) | |
| const ipResponse = await fetch( | |
| ` | |
| ) | |
| if (!ipResponse.ok) { | |
| res.status(200).json(fallbackLocation) | |
| return | |
| } | |
| const apiResponse: ApiResponse = await ipResponse.json() | |
| if (!apiResponse.loc) { | |
| res.status(200).json(fallbackLocation) | |
| return | |
| } | |
| const [lat, long] = apiResponse.loc.split(',') | |
| const data: Data = { | |
| latitude: parseFloat(lat), | |
| longitude: parseFloat(long), | |
| } | |
| res.status(200).json(data) | |
| } catch (error) { | |
| res.status(200).json(fallbackLocation) | |
| } | |
| } |
Footnotes
-
CWE-755: Improper Handling of Exceptional Conditions - https://cwe.mitre.org/data/definitions/755.html ↩
There was a problem hiding this comment.
Code Review
This pull request introduces a new 3D Map page and search action, alongside refactoring the IP lookup API to improve client IP detection and fallback handling. The review feedback highlights two important issues: a hardcoded API token in the IP lookup endpoint that should be moved to an environment variable, and a React useEffect dependency issue in the map component that causes the map to be unnecessarily destroyed and recreated on location updates.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const ipResponse = await fetch( | ||
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | ||
| ) |
There was a problem hiding this comment.
The IPInfo API token is hardcoded in the fetch URL. Hardcoding API keys or tokens in source code is a security risk and makes it difficult to rotate keys. Please move this token to an environment variable (e.g., process.env.IPINFO_TOKEN) and access it securely.
| const ipResponse = await fetch( | |
| `https://ipinfo.io/${clientIp}/json?token=ef8461623ce04a`, | |
| ) | |
| const token = process.env.IPINFO_TOKEN || 'ef8461623ce04a' | |
| const ipResponse = await fetch( | |
| `https://ipinfo.io/${clientIp}/json?token=${token}`, | |
| ) |
| useEffect(() => { | ||
| if ( | ||
| !isMapLibreReady || | ||
| !location || | ||
| !mapContainerRef.current || | ||
| mapRef.current | ||
| ) { | ||
| return | ||
| } | ||
|
|
||
| const maplibregl = window.maplibregl | ||
|
|
||
| if (!maplibregl) { | ||
| setStatus('Map library failed to load') | ||
| return | ||
| } | ||
|
|
||
| const center: [number, number] = [ | ||
| location.coordinates.longitude, | ||
| location.coordinates.latitude, | ||
| ] | ||
| const map = new maplibregl.Map({ | ||
| container: mapContainerRef.current, | ||
| style: MAP_STYLE, | ||
| center, | ||
| zoom: 16, | ||
| pitch: 60, | ||
| bearing: -20, | ||
| antialias: true, | ||
| attributionControl: { compact: true }, | ||
| }) | ||
|
|
||
| map.addControl( | ||
| new maplibregl.NavigationControl({ visualizePitch: true }), | ||
| 'top-right', | ||
| ) | ||
| map.addControl( | ||
| new maplibregl.GeolocateControl({ | ||
| positionOptions: browserLocationOptions, | ||
| trackUserLocation: true, | ||
| showUserHeading: true, | ||
| }), | ||
| 'top-right', | ||
| ) | ||
| map.addControl(new maplibregl.FullscreenControl(), 'top-right') | ||
|
|
||
| markerRef.current = new maplibregl.Marker({ color: '#38bdf8' }) | ||
| .setLngLat(center) | ||
| .addTo(map) | ||
| mapRef.current = map | ||
|
|
||
| return () => { | ||
| markerRef.current?.remove() | ||
| map.remove() | ||
| markerRef.current = null | ||
| mapRef.current = null | ||
| } | ||
| }, [isMapLibreReady, location]) |
There was a problem hiding this comment.
The useEffect hook that initializes the map has location in its dependency array. In React, even if you return early using mapRef.current, the cleanup function from the previous execution of the effect is run before the next execution. This means that whenever location changes, the map is destroyed (map.remove()) and recreated from scratch, causing a poor user experience and unnecessary API/tile requests.
To fix this, you should separate the map initialization from location updates. You can track whether the location has been loaded using a boolean flag (e.g., hasLocation = location !== null) and use that as a dependency instead of the location object itself. This ensures the map is initialized only once when the location first becomes available, and subsequent location updates are handled smoothly by the other useEffect hook.
const hasLocation = location !== null
const locationRef = useRef(location)
useEffect(() => {
locationRef.current = location
}, [location])
useEffect(() => {
if (
!isMapLibreReady ||
!hasLocation ||
!mapContainerRef.current ||
mapRef.current
) {
return
}
const maplibregl = window.maplibregl
if (!maplibregl || !locationRef.current) {
setStatus('Map library failed to load')
return
}
const center: [number, number] = [
locationRef.current.coordinates.longitude,
locationRef.current.coordinates.latitude,
]
const map = new maplibregl.Map({
container: mapContainerRef.current,
style: MAP_STYLE,
center,
zoom: 16,
pitch: 60,
bearing: -20,
antialias: true,
attributionControl: { compact: true },
})
map.addControl(
new maplibregl.NavigationControl({ visualizePitch: true }),
'top-right',
)
map.addControl(
new maplibregl.GeolocateControl({
positionOptions: browserLocationOptions,
trackUserLocation: true,
showUserHeading: true,
}),
'top-right',
)
map.addControl(new maplibregl.FullscreenControl(), 'top-right')
markerRef.current = new maplibregl.Marker({ color: '#38bdf8' })
.setLngLat(center)
.addTo(map)
mapRef.current = map
return () => {
markerRef.current?.remove()
map.remove()
markerRef.current = null
mapRef.current = null
}
}, [isMapLibreReady, hasLocation])
Motivation
Description
pages/map.tsxthat loads MapLibre from CDN, initializes a pitched/3D map (pitch,bearing,antialias), adds navigation/geolocate/fullscreen controls, and places a marker at the resolved center.getBrowserLocation) with an async fallback toGET /api/ip-lookup(getIpLocation) and a final default center when both fail.pages/api/ip-lookup.tsto read common forwarding headers (x-forwarded-for,cf-connecting-ip), handle missinglocfrom the IP provider, and return a safefallbackLocation.components/search/Actions.tsxto include an entry for/map.Testing
npm run lint, which completed with non-blocking warnings.npm run build, which completed successfully and produced an optimized build./map, which returned200 OKand the expected page HTML including the3D Map Wake Locktitle and MapLibre stylesheet reference.Codex Task