fix(api): answer 404 instead of 500 for an unknown onboard organization - #290
Merged
Merged
Conversation
GET /v1/onboard/driver-onboard-settings/{companyId} passed the result of
findCompanyByPublicId() straight into driverOnboardSetting() without checking
it. The lookup is declared ?Company and returns null for any public id that
does not resolve, so null->uuid yielded null, which then hit the string type
declaration on driverOnboardSetting() and threw:
TypeError: driverOnboardSetting(): Argument #1 ($companyUuid) must be of
type string, null given
That escaped as an unhandled exception and rendered a ~1.2 MB HTML stack
trace with a 500. Reproduced against a live stack with both a bogus public id
and the literal "{{organization_id}}" the Postman collection was sending; a
valid public id was unaffected.
An unknown organization is a client error, so guard the null and return 404
through a new errorResponse() seam, matching the protected-seam style the rest
of this controller already uses.
The existing probe overrode findCompanyByPublicId() to always return a
hydrated Company, so the null branch was unreachable in tests. It can now be
made to miss, and a regression test pins the 404 and its error envelope.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## dev-v0.6.60 #290 +/- ##
===============================================
Coverage 100.00% 100.00%
- Complexity 9766 9768 +2
===============================================
Files 521 521
Lines 37762 37766 +4
===============================================
+ Hits 37762 37766 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
This was referenced Aug 8, 2026
Merged
Open
roncodes
marked this pull request as ready for review
August 10, 2026 05:26
codecov failed on this branch alone. The 404 assertion added in
SmallControllerContractsTest goes through FleetOpsPublicNavigatorControllerProbe,
which overrides errorResponse() to keep the fixture small — so the real
one-liner never executed and the new seam was the only uncovered code in the
file.
The other seams in this controller are covered by the SQLite-backed navigator
test in GeofenceDwellAndBulkNotifyTest, which drives the real controller, so
the missing-company case belongs there too.
Only the status is asserted. The harness `response()` shim envelopes errors as
{"error": ...} while the core-api macro it stands in for produces
{"errors": [...]}; the status code is the part that holds in both, and the
payload shape is already pinned by the probe-based test.
Verified by slice: NavigatorController has no uncovered lines.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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
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.
Problem
GET /v1/onboard/driver-onboard-settings/{companyId}returned HTTP 500 with a ~1.2 MB HTML stack trace.getDriverOnboardSettings()passedfindCompanyByPublicId()straight intodriverOnboardSetting()without checking it. The lookup is declared?Company— the null return is deliberate — but the call site never guarded it, sonull->uuidyieldednull, which hit thestringtype declaration and threw:Reproduced against a live stack:
company_hfUOJNWBuOcompany_DOESNOTEXISTtext/html, 1,190,813 b{{organization_id}}text/html, 1,193,751 bThe failure was independent of the unresolved Postman variable — any public id that does not resolve produced it.
Change
Guard the null and return 404 via a new
errorResponse()seam, matching the protected-seam style the rest of this controller already uses (jsonResponse,findCompanyByPublicId,driverOnboardSetting).Test coverage
FleetOpsPublicNavigatorControllerProbeoverrodefindCompanyByPublicId()to always return a hydratedCompany, so the null branch was unreachable — which is why this shipped. The probe can now be made to miss, and a regression test pins the 404 and its error envelope.Related
The HTML-instead-of-JSON rendering is a separate, broader defect in the API exception handler — any unlisted exception on any
/v1/*route leaks the same way (confirmed onPUT /v1/orders). Fixed separately in fleetbase/core-api.Verification status
To verify once a stack is up:
The unit test also has not been run locally — it needs a real 542 MB
server_vendorcopy in the worktree.🤖 Generated with Claude Code