feat: add input validation and error handling to geocoder endpoint

[Prince-Prajapati07]

Changes

• Added input validation for the query parameter
• Added error handling for missing bounds
• Improved error responses with proper HTTP status codes
• Added try-catch for geocoding operations

Testing

• Tested with missing query parameter (400)
• Tested with unavailable bounds (503)
• Needs testing with valid API key (blocked by Bug: Add input validation and error handling to geocoder endpoints #320)

Closes #320

[CLAassistant]

All committers have signed the CLA.

[Copilot]

Pull request overview

Implements structured input validation and error handling for the /api/oba/geocode-location endpoint to improve clarity of failures and HTTP status codes.

Changes:

• Validates the query search parameter and responds with a 400 error and structured payload when missing.
• Ensures geocoding only runs when bounds are available, returning a 503 error when the bounds cache is not ready.
• Wraps the geocoding call in a try/catch and normalizes unexpected failures to a 500 error with a JSON error body.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The local validation in locationSearch that throws a plain Error for a missing query is inconsistent with the 400 response produced by the GET handler above, and in practice is unreachable for this route because GET already rejects empty queries. To keep error handling consistent and easier to reason about, consider removing this redundant check or converting it to use the same error(400, { ... }) pattern (and ensuring it’s not double-applied) if locationSearch is expected to be reused elsewhere.

Suggested change

	if (!query) {
	throw new Error('Query parameter is required');
	}

[Copilot]

Inside the try block you throw error(500, { ... details: 'No results returned from geocoder' }) when locationResponse is falsy, but the surrounding catch immediately catches all errors (including that HttpError) and rethrows a new error(500, { ... details: err.message }). This means the more specific 'No results returned from geocoder' detail is never surfaced to clients, and all geocoding failures collapse to the same generic 500 payload; consider either letting existing HttpErrors propagate unchanged or preserving their body/details when rethrowing so that distinct failure modes remain observable.

[aaronbrethorst]

Hey Prince, thanks for adding proper validation and error handling to the geocoder endpoint — this kind of defensive server-side code is important for giving callers meaningful error responses instead of opaque 500s. The structure of your error responses (with message, code, and details) is well thought out. Before we can merge this, I will need you to make a couple changes:

Critical

1. File uses 4-space indentation instead of tabs

The entire file uses 4-space indentation, but the project uses tabs (check any sibling file in src/routes/api/oba/). This causes npm run lint to fail. Additionally, the file is missing a trailing newline.

Running npm run format will fix both issues.

2. The catch block swallows SvelteKit HttpErrors

In src/routes/api/oba/geocode-location/+server.js:55-61, the catch block catches all errors, including the HttpError thrown by throw error(500, {...}) on line 42. This means:

• The structured error body from line 42 (details: 'No results returned from geocoder') is lost
• The caught HttpError gets re-wrapped in a new 500 with details: err.message (which would be empty or unhelpful for an HttpError)

The correct pattern already exists in this codebase — see src/routes/api/otp/plan/+server.js:76-83:

catch (err) {
    if (err.status) throw err;  // Re-throw SvelteKit HttpErrors

    throw error(500, {
        message: 'Failed to process geocoding request',
        code: 'GEOCODING_ERROR',
        details: err.message
    });
}

Adding if (err.status) throw err; as the first line of the catch block lets SvelteKit HttpErrors (like the 400 and 503 you set up earlier, or the inner 500) propagate correctly, while still catching unexpected errors from the geocoder service.

Important

3. Redundant !query validation in locationSearch()

The !query check in locationSearch() (lines 10-12) is redundant because the GET handler already validates searchInput on lines 24-29 before passing it to locationSearch(). Since locationSearch is only called from GET, the query will never be falsy when it reaches line 10. I'd remove the check from locationSearch() to keep the validation in one place.

Fit and Finish

4. Consider whether the !bounds 503 is appropriate

The getBoundsCache() null check on lines 32-37 returns a 503. While defensible, none of the sibling endpoints (place-suggestions, search) check for null bounds — they pass it through to the geocoder, which can still return results (just without location bias). Since this endpoint is used for trip planning where users type explicit locations, a null bounds may not warrant blocking the request. Consider whether passing null bounds through (like the sibling endpoints) would be a better UX.

If you want to keep the 503, that's fine — just wanted to flag the inconsistency.

Thanks again, and I look forward to merging this change.

[Prince-Prajapati07]

Hey, thanks for the detailed review @aaronbrethorst sir!

I’ve pushed updates for everything you mentioned- ran npm run format for tabs/newline, updated the catch block to rethrow SvelteKit HttpErrors, removed the redundant !query check, and kept the bounds 503 for now (noted your UX suggestion).

Let me know when you get a chance to re-review, happy to tweak anything else. Thanks!