Distinguish request vs response source on JsonSchemaException (#369)#370
Merged
koriym merged 7 commits intoMay 28, 2026
Merged
Conversation
|
Important Review skippedAuto reviews are disabled on this repository. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
WalkthroughJSON Schema検証例外の出力元を識別可能にするため、 ChangesJSON Schema 例外の階層化と実装
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## 1.x #370 +/- ##
===========================================
Coverage 100.00% 100.00%
- Complexity 683 688 +5
===========================================
Files 94 96 +2
Lines 1707 1720 +13
===========================================
+ Hits 1707 1720 +13 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
…nday#369) Two concrete subclasses of JsonSchemaException let direct-catch code discriminate between client-supplied bad input and a resource producing off-schema output: - JsonSchemaRequestException — request parameter validation failed (Code::BAD_REQUEST, 400) - JsonSchemaResponseException — response body validation failed (Code::ERROR, 500) `final` is dropped from the parent so subclassing is permitted. The parent constructor and getErrors() signature stay unchanged, so existing `catch (JsonSchemaException $e)` keeps working and structured errors flow through subclasses automatically (bearsunday#367's JsonSchemaErrors ride along via inheritance). Interceptor refactor: - validate() split into validateAsRequest() / validateAsResponse(), each throws its specific subclass directly — no class-string dance. - Shared logic split into runValidator() (justinrainbow invocation) and buildValidationFailure() (message + structured errors from a failed validator). - catch sites narrowed to the concrete subclass so no `assert($e instanceof ...)` is needed. Safe because JsonSchemaNotFoundException extends LogicException directly, not JsonSchemaException, so it bubbles past the catch. Handler interfaces (`JsonSchemaRequestExceptionHandlerInterface`, `JsonSchemaExceptionHandlerInterface`) get a PHPDoc `@param` narrowed to the concrete subtype the interceptor actually delivers. Runtime parameter types stay on the parent for BC. Closes bearsunday#369. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
koriym
force-pushed
the
feature/369-request-response-exception-source
branch
from
96a9cb7 to
9bf68a9
Compare
BEAR.Resource is a resource framework, not an HTTP framework. Pinning `Code::BAD_REQUEST` / `Code::ERROR` onto the exception subclasses in the docs read as if the class itself enforced the code — it doesn't. The interceptor chooses those codes when throwing; subclasses inherit the parent constructor unchanged and accept any code. Same separation of concerns the interceptor itself observes: the subclass carries the *semantic* (request vs response source), the caller carries the *runtime value* (which numeric code to use). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Adds two `@psalm-type` aliases in `BEAR\Resource\Types`:
@psalm-type ClientErrorCode = int<400, 499>
@psalm-type ServerErrorCode = int<500, 599>
Each subclass overrides the parent constructor to narrow `@param int $code`
to the appropriate range:
- JsonSchemaRequestException::__construct @param ClientErrorCode $code
(defaults to Code::BAD_REQUEST)
- JsonSchemaResponseException::__construct @param ServerErrorCode $code
(defaults to Code::ERROR)
The point: BEAR.Resource is a resource framework, not an HTTP framework.
Whether a schema-validation request failure should map to 400, 422, or
some other 4xx isn't something the exception class can authoritatively
decide. Pinning a single value would lock the choice; using a wide
`int` would give callers no guidance. A range type lets the caller pick
the right code per their context while the type system blocks
nonsensical values (200, 999) statically.
Runtime behaviour: defaults unchanged. Interceptor still throws with
`Code::BAD_REQUEST` / `Code::ERROR`.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The previous entries read like a design diary — names of internal types, narrowing rationale, alternative approaches considered. None of that helps a downstream consumer decide whether to upgrade or how. Two lines each: what's new, where to read more. The richer rationale already lives in the PR descriptions (bearsunday#364, bearsunday#369) and the README sections. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Replace three paragraphs of justification with one code block. The
comments in the catch arms carry the same information (which validation
failed + the type-constrained `$code` range) without the prose
overhead. The BC fact ("parent catch matches both") stays as a one-liner.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
「リクエスト・レスポンス由来の区別」suggests "discrimination of request-response origin" which is unnatural — `由来` qualifies events, not the act of discriminating. Use a slash separator instead. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@README.ja.md`:
- Around line 468-469: Replace the PHPDoc-style inline ranges in the catch
comments for JsonSchemaRequestException and JsonSchemaResponseException with
plain-language descriptions; for example change the comment on
JsonSchemaRequestException to "リクエスト検証失敗（HTTPステータス 400–499）" and the comment on
JsonSchemaResponseException to "レスポンス検証失敗（HTTPステータス 500–599）" so the intent is
clear without using `@param` int<...> syntax.
In `@README.md`:
- Around line 677-678: Replace the PHPDoc-style inline comments after the catch
blocks for JsonSchemaRequestException and JsonSchemaResponseException with
plain-language status code ranges; specifically change the comment for
JsonSchemaRequestException to something like "request validation failed — status
codes 400–499" and for JsonSchemaResponseException to "response validation
failed — status codes 500–599" so the intent is clear without using `@param`
PHPDoc syntax.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: fad71f08-dd1f-4ede-9630-f6c679a3a820
📒 Files selected for processing (13)
CHANGELOG.mdREADME.ja.mdREADME.mdsrc/JsonSchema/Exception/JsonSchemaException.phpsrc/JsonSchema/Exception/JsonSchemaRequestException.phpsrc/JsonSchema/Exception/JsonSchemaResponseException.phpsrc/JsonSchema/Interceptor/JsonSchemaInterceptor.phpsrc/JsonSchema/JsonSchemaExceptionHandlerInterface.phpsrc/JsonSchema/JsonSchemaRequestExceptionHandlerInterface.phpsrc/Types.phptests/JsonSchema/Exception/JsonSchemaRequestExceptionTest.phptests/JsonSchema/Exception/JsonSchemaResponseExceptionTest.phptests/Module/JsonSchemaModuleTest.php
`runValidator` was named that only because the original `validate()` slot was taken. After splitting into `validateAsRequest()` / `validateAsResponse()`, the slot is free and `validate()` is the natural name for "run the validator and hand back its state". Also fold in CodeRabbit's README nit: replace the inline `@param int<400, 499>` / `@param int<500, 599>` (PHPDoc syntax leaking into a code comment) with plain `(4xx)` / `(5xx)`. The precise range types stay in their canonical place (`src/Types.php` + the subclass constructors). Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
Member
Author
|
@coderabbitai approve |
✅ Actions performedComments resolved and changes approved. |
koriym
added a commit
that referenced
this pull request
May 28, 2026
Reflect the additions from #367 (structured errors via JsonSchemaErrors collection, JsonSchemaError DTO, render/format placeholder mechanism) and #370 (JsonSchemaRequestException / JsonSchemaResponseException subclasses with ClientErrorCode / ServerErrorCode range types). - llms.txt: extend the JsonSchemaModule entry to surface the exception subclasses and the structured-errors collection so LLM consumers notice the API without reading source. - llms-full.txt: same one-liner (preserves the strict-superset rule), plus two new subsections in JSON Schema Validation covering the request/response discrimination, range-typed @param, structured error access pattern, and the schema-side errorMessage lookup. Add two notes to the stale-assumptions list. Co-Authored-By: Claude Opus 4.7 <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
1 participant
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.
Closes #369
Important
Stacks on top of #367 — please merge #367 first. Until #367 lands, the diff against
1.xwill show #367's changes too. After #367 merges, this PR will rebase to only show #369.cc @koriym
Summary
Two concrete subclasses of
JsonSchemaExceptionlet direct-catch code discriminate client-supplied bad input from off-schema output:JsonSchemaRequestException— request-parameter validation failed (Code::BAD_REQUEST, 400)JsonSchemaResponseException— response-body validation failed (Code::ERROR, 500)Design notes
finaldropped from the parent; nothing else about the parent changes. Existingcatch (JsonSchemaException $e)still matches both subclasses — BC safe.validate()split intovalidateAsRequest()/validateAsResponse(). Each throws its specific subclass directly — noclass-stringindirection orif/elseover the target type.runValidator()(justinrainbow invocation) +buildValidationFailure()returningarray{string, JsonSchemaErrors}for the message + structured-error pair.assert($e instanceof ...)needed. Safe becauseJsonSchemaNotFoundExceptionextendsLogicExceptiondirectly, notJsonSchemaException, so it bubbles past the catch.@paramPHPDoc narrowed to the concrete subtype the interceptor actually delivers. Runtime parameter type stays on the parent for BC with existing implementations.Design history
The class-string approach was tried first (codex's initial pass). Switched to the split-method shape after review — codex itself agreed on the second pass: "split is cleaner, narrow catch is sound, two-helper split is natural".
CI
Test plan
vendor/bin/phpunitvendor/bin/phpcs --standard=phpcs.xml src testsvendor/bin/phpstan --memory-limit=-1 analyse -c phpstan.neonvendor/bin/psalm --monochrome --threads=1 --no-cachevendor-bin/tools/vendor/bin/phpmd --exclude src/Annotation src text ./phpmd.xml🤖 Generated with Claude Code