Fix swallowed duplicated trip lookup errors

[priyanshu7739410]

Description

Fixes #1366.

When resolving a DUPLICATED trip, sql.ErrNoRows is expected and allows the handler to fall back to the stripped trip ID. However, other database errors were being treated like a not-found result.

This change propagates unexpected database errors from both the direct and fallback trip lookups so the endpoint returns HTTP 500 instead of silently continuing.

Tests

• Added regression coverage for direct lookup database errors.
• Added regression coverage for fallback lookup database errors.
• Added coverage confirming legitimate suffix-stripped fallback still succeeds.
• go test -tags "purego" ./internal/restapi/...
• go vet -tags "purego" ./...
• git diff --check

Summary by CodeRabbit

• Bug Fixes
  • Improved handling of duplicated trips when resolving their base-trip information.
  • Database lookup failures now return a clear server error instead of silently continuing with incomplete results.
  • Duplicated trips remain available when resolution is unsuccessful, preserving their original identifiers.
  • Added support for successful fallback resolution when duplicated trip identifiers use numeric suffixes.
  • Improved response consistency for duplicated-trip lookup failures and fallback scenarios.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Repository UI

Review profile: ASSERTIVE

Plan: Pro Plus

Run ID: aace667a-966f-47cf-960f-e68aeaa8e643

📥 Commits

Reviewing files that changed from the base of the PR and between e01e1a9 and 60251a6.

📒 Files selected for processing (2)

• internal/restapi/trips_for_route_handler.go
• internal/restapi/trips_for_route_handler_test.go

Included review availability: Your plan provides up to 1 included review per hour; 0 remain after this review.

---

📝 Walkthrough

Walkthrough

The DUPLICATED trip resolver now returns non-not-found database errors. The trips-for-route handler responds with HTTP 500 for those errors. Tests cover direct lookup failure, stripped-ID lookup failure, and successful suffix fallback resolution.

Changes

DUPLICATED trip error handling

Layer / File(s)	Summary
Resolver error propagation internal/restapi/trips_for_route_handler.go	resolveDuplicatedBaseTrip returns lookup errors. The handler sends a server-error response when resolution fails.
Duplicated trip test support and validation internal/gtfs/gtfs_manager_mock.go, internal/restapi/trips_for_route_handler_test.go	Test support can add duplicated vehicles safely. Tests cover database failures and numeric-suffix fallback resolution.

Estimated code review effort: 3 (Moderate) | ~20 minutes

Merge Risk: ⚪ Minimal · up to 60251

Unexpected database failures now return the endpoint’s existing generic HTTP 500 response instead of being treated as missing data, while valid fallback lookups continue to succeed. No actionable merge-blocking risk remains after normal checks and review.

Sequence Diagram(s)

sequenceDiagram
  participant TripsForRouteHandler
  participant resolveDuplicatedBaseTrip
  participant GTFSDatabase
  participant serverErrorResponse
  TripsForRouteHandler->>resolveDuplicatedBaseTrip: Resolve duplicated trip ID
  resolveDuplicatedBaseTrip->>GTFSDatabase: GetTrip full or stripped ID
  GTFSDatabase-->>resolveDuplicatedBaseTrip: Return trip, not found, or database error
  resolveDuplicatedBaseTrip-->>TripsForRouteHandler: Return resolved trip or error
  TripsForRouteHandler->>serverErrorResponse: Send HTTP 500 for database error

Loading

Suggested reviewers: 3rabiii, ahmedhossamdev, arcoder181105

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Docstring Coverage	⚠️ Warning	Docstring coverage is 66.67% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 3 functions across 3 files.	Write docstrings for the functions missing them to satisfy the coverage threshold.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The title clearly identifies the main change: preventing duplicated trip lookup errors from being swallowed.
Linked Issues check	✅ Passed	The changes satisfy issue #1366. They propagate non-sql.ErrNoRows errors from both direct and suffix-stripped lookups, return HTTP 500 responses, preserve the fallback for sql.ErrNoRows, and add regre…
Out of Scope Changes check	✅ Passed	The changes remain within scope. The mock helper supports the new regression fixtures, and the handler and tests directly address duplicated trip lookup error propagation.

Full details: Linked Issues check

Explanation

The changes satisfy issue #1366. They propagate non-sql.ErrNoRows errors from both direct and suffix-stripped lookups, return HTTP 500 responses, preserve the fallback for sql.ErrNoRows, and add regression tests for both failure paths and successful fallback resolution.

• Fix all pre-merge checks with AI

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands.}

[burma-shave]

merge conflicts

[priyanshu7739410]

Thanks, I am working on resolving the merge conflicts now. I will push the updated changes once they are resolved.

[sonarqubecloud]

Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
0.0% Coverage on New Code
0.0% Duplication on New Code

See analysis details on SonarQube Cloud

[priyanshu7739410]

Merge conflicts have been resolved and all CI checks are passing. I also verified the changes locally.