Skip to content

Improve Windows PowerShell curl instructions for health check , Fix i…#770

Closed
jeyamoorthi wants to merge 1 commit intoOneBusAway:mainfrom
jeyamoorthi:fix/agency-404-error
Closed

Improve Windows PowerShell curl instructions for health check , Fix i…#770
jeyamoorthi wants to merge 1 commit intoOneBusAway:mainfrom
jeyamoorthi:fix/agency-404-error

Conversation

@jeyamoorthi
Copy link
Copy Markdown

Summary

Fixes incorrect HTTP status code when an agency is not found.

Problem

The API returned a 500 Internal Server Error when sql.ErrNoRows occurred while fetching an agency.

Solution

Added explicit handling for sql.ErrNoRows and return a 404 Not Found response.

Impact

  • Aligns with REST API standards
  • Improves client-side error handling
  • Prevents misleading server error reporting

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented Mar 22, 2026

CLA assistant check
All committers have signed the CLA.

@FLASH2332
Copy link
Copy Markdown
Contributor

Hey, just curious — does it make sense to think about what this returns when the endpoint is behind auth? I've read that some APIs avoid returning a specific 404 in those cases to not reveal whether a resource actually exists. Wondering if that applies here!

@jeyamoorthi
Copy link
Copy Markdown
Author

jeyamoorthi commented Mar 22, 2026 via email

@tejasva-vardhan
Copy link
Copy Markdown
Contributor

Great discussion here! @FLASH2332 brings up a very valid API security pattern regarding resource enumeration.

However, I completely agree with @jeyamoorthi's assessment in this specific context. For OneBusAway/Maglev, the API keys are primarily used for rate limiting, quotas, and usage tracking rather than strict data-level authorization. Since GTFS agency data is inherently public transit information, returning a standard 404 Not Found for an invalid agency_id is the correct RESTful approach. It's exactly what downstream clients need to gracefully handle missing feeds instead of crashing on a generic 500 Internal Server Error.

Good catch and solid fix on handling the sql.ErrNoRows gracefully! 🚀

@ARCoder181105
Copy link
Copy Markdown
Contributor

@jeyamoorthi

The title of this PR seems inaccurate can you changes that to something that makes sense,

Like -> fix : http-status-response etc..

OR any other related title helps , it is help full for documenting the changes...

@aaronbrethorst
Copy link
Copy Markdown
Member

superseded by #772

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants