Skip to content

Let observer refusals reach the caller on update and bulk delete - #259

Merged
roncodes merged 2 commits into
release/v1.6.62from
feature/github-issue-256-review-3717b2
Sep 16, 2026
Merged

roncodes merged 2 commits into
release/v1.6.62from
feature/github-issue-256-review-3717b2

Conversation

@roncodes

Copy link
Copy Markdown
Member

Fixes #256.

An observer that refuses a write by throwing FleetbaseRequestValidationException had its explanation discarded on two paths. The write was still refused, but the caller saw Invalid request or Error occurred while trying to update a Widget instead of the message the observer wrote. Both sites already had a handler for this exception; each was made unreachable by a catch block one frame deeper.

What changed

  • HasApiModelBehavior::updateRecordFromRequest() catches FleetbaseRequestValidationException ahead of its catch-all and rethrows it untouched, so HasApiControllerBehavior::updateRecord() and the global exception handler can render getErrors(). The catch-all keeps wrapping every other exception exactly as before, so database and internal errors still do not leak in production.
  • HasApiControllerBehavior::bulkDelete() now catches FleetbaseRequestValidationException, then QueryException, then \Exception. The two specific catches were previously dead code behind the catch-all and were wrapped in a @codeCoverageIgnore block acknowledging that; the block is gone because the branches are now reachable and covered.
  • RELEASE.md gains a Fixes section for this change.

Create and single-delete paths were already correct and are unchanged.

Tests

  • HasApiModelBehaviorTest: a model whose updating event throws the exception propagates the same instance with getErrors() intact under both app.debug settings, and the row is left unmodified.
  • HasApiControllerBehaviorTest: updateRecord() and bulkDelete() return the observer's errors array, and bulkDelete() surfaces a QueryException message through its now-reachable catch.

Both new tests fail against the previous code and pass with the fix.

Validation

  • composer test:lint passes.
  • composer test:date-drift passes.
  • phpstan reports no errors on the two changed source files.
  • Full suite with coverage passes and the 100% line, method and class floor holds.

An observer that refuses a write by throwing
FleetbaseRequestValidationException had its explanation discarded on two
paths. The write was still refused, but the caller saw "Invalid request"
or "Error occurred while trying to update a X" instead of the message
the observer wrote. Both sites already had a handler for the exception;
each was made unreachable by a catch block one frame deeper.

updateRecordFromRequest() wrapped every exception from $record->update()
in a plain \Exception, so the controller's dedicated catch and the global
handler's getErrors() rendering never saw the original. It now rethrows
FleetbaseRequestValidationException untouched and keeps wrapping
everything else, so database and internal errors still do not leak.

bulkDelete() listed catch (\Exception) before its QueryException and
FleetbaseRequestValidationException catches, leaving both dead behind a
@codeCoverageIgnore block that acknowledged as much. The catches are now
ordered most specific first and the ignore block is gone because the
branches are reachable and covered.

Create and single delete were already correct and are unchanged.

Fixes #256
@roncodes
roncodes force-pushed the feature/github-issue-256-review-3717b2 branch from d3dd3b2 to 09af864 Compare September 16, 2026 04:08
@codecov

codecov Bot commented Sep 16, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 100.00%. Comparing base (546d962) to head (09af864).

Additional details and impacted files
@@                 Coverage Diff                 @@
##             release/v1.6.62      #259   +/-   ##
===================================================
  Coverage             100.00%   100.00%           
- Complexity              6750      6751    +1     
===================================================
  Files                    398       398           
  Lines                  22505     22511    +6     
===================================================
+ Hits                   22505     22511    +6     
Flag Coverage Δ
backend 100.00% <100.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@roncodes
roncodes merged commit f723449 into release/v1.6.62 Sep 16, 2026
5 checks passed
@roncodes
roncodes deleted the feature/github-issue-256-review-3717b2 branch September 16, 2026 04:23
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.

1 participant