Skip to content

Feature/problem details - #1

Closed
ljonesfl wants to merge 3 commits into
developfrom
feature/problem-details
Closed

ljonesfl wants to merge 3 commits into
developfrom
feature/problem-details

Conversation

@ljonesfl

@ljonesfl ljonesfl commented Jan 7, 2026 •

Copy link
Copy Markdown
Member

Note

Implements standardized API error responses and supporting exceptions.

  • Adds ProblemDetails (RFC 9457) with validation, extension fields, toArray(), jsonSerialize(), and fromArray()
  • Introduces ProblemType enum with common error URIs, default titles, recommended status codes, and toProblemDetails() factory
  • Adds Unauthorized (realm support) and Forbidden (resource/permission context) exceptions
  • Comprehensive unit tests for ProblemDetails, ProblemType, and the new exceptions

Written by Cursor Bugbot for commit 1d62d13. This will update automatically on new commits. Configure here.

Summary by CodeRabbit

  • New Features

    • RFC 9457-compliant standardized API error responses with validation and JSON serialization
    • Enhanced error/exception types exposing authentication realm, resource and permission context
    • New typed problem categories with recommended HTTP status codes and default titles; convenient builders for error responses
  • Tests

    • Comprehensive test suites covering new error types, serialization, validation, and enum integrations

✏️ Tip: You can customize this high-level summary in your review settings.

@coderabbitai

coderabbitai Bot commented Jan 7, 2026 •

Copy link
Copy Markdown
Contributor
📝 Walkthrough

Walkthrough

Adds HTTP error handling: two exceptions (Forbidden 403, Unauthorized 401), an RFC-9457 ProblemDetails class with validation/serialization, a ProblemType enum mapping types to statuses and default titles, and comprehensive PHPUnit tests for these components.

Changes

Cohort / File(s) Summary
Exception classes
src/Core/Exceptions/Forbidden.php, src/Core/Exceptions/Unauthorized.php
New exception classes: Forbidden (stores optional resource and permission, default code 403) and Unauthorized (stores optional realm, default code 401). Both add constructors and getters.
Problem Details core
src/Core/ProblemDetails/ProblemDetails.php
New RFC 9457-compliant ProblemDetails class with constructor, validation (type/title/status/extensions), accessors, toArray(), jsonSerialize(), and static fromArray() reconstruction.
ProblemType enum
src/Core/ProblemDetails/ProblemType.php
New ProblemType enum listing standardized problem URIs, getRecommendedStatus(), getDefaultTitle(), and toProblemDetails() helper to build ProblemDetails.
Unit tests
tests/Core/Exceptions/ForbiddenTest.php, tests/Core/Exceptions/UnauthorizedTest.php, tests/ProblemDetailsTest.php
New PHPUnit suites covering constructors, getters, serialization, validation error cases, enum integration, and exception behavior (chaining, inheritance, nullability).
Docs / changelog
versionlog.md
Updated version log entry reflecting introduced abstractions and changes.

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Poem

🐇 I hopped through code, nibbling bugs away,
Added reasons for errors, made meanings stay.
Forbidden gates and realms to show,
Problems detailed in tidy row.
A rabbit's nod — now APIs know.

🚥 Pre-merge checks | ✅ 1 | ❌ 2
❌ Failed checks (1 warning, 1 inconclusive)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 56.45% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
Title check ❓ Inconclusive The title 'Feature/problem-details' is vague and generic, using a feature branch naming convention rather than describing the actual changes. Replace with a specific description of the main change, e.g., 'Add RFC 9457 ProblemDetails implementation with auth exceptions' to clearly convey what was implemented.
✅ Passed checks (1 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing touches
  • 📝 Generate docstrings

📜 Recent review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between fb595d5 and 1d62d13.

📒 Files selected for processing (1)
  • tests/ProblemDetailsTest.php
🚧 Files skipped from review as they are similar to previous changes (1)
  • tests/ProblemDetailsTest.php
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cursor Bugbot

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

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 2

🤖 Fix all issues with AI agents
In @tests/ProblemDetailsTest.php:
- Around line 1-7: The test namespace is inconsistent: change the namespace
declaration from "Neuron\Core\Tests" to follow the project's test convention
(e.g., "Tests\Core" or "Tests\Core\ProblemDetails" to match other test files);
update the namespace in the ProblemDetailsTest class accordingly so it matches
PSR-4 autoloading conventions used by other tests (ensure the class name
ProblemDetailsTest and its use/imports remain unchanged).

In @versionlog.md:
- Around line 1-2: Add descriptive changelog entries under the "## 0.8.5"
section documenting the new public features: include a bullet for the new
Forbidden and Unauthorized exception classes (mention their purpose for
auth/authorization errors), a bullet for the ProblemDetails class noting it
implements RFC 9457 for standardized error payloads, and a bullet for the
ProblemType enum listing the included standard error types; keep each entry
concise and user-facing so consumers understand what changed and how to use
Forbidden, Unauthorized, ProblemDetails, and ProblemType.
🧹 Nitpick comments (1)
tests/Core/Exceptions/UnauthorizedTest.php (1)

93-108: Consider simplifying the test structure (optional).

The test works correctly but could be simplified by removing the expectException and re-throw, keeping only the assertions in the catch block. However, the current implementation is valid and the additional complexity is minimal.

♻️ Simplified alternative
 	public function testExceptionWithRealmThrowable(): void
 	{
-		$this->expectException( Unauthorized::class );
-
 		try
 		{
 			throw new Unauthorized( 'Access denied', 'Secure Zone' );
 		}
 		catch( Unauthorized $e )
 		{
 			$this->assertEquals( 'Access denied', $e->getMessage() );
 			$this->assertEquals( 'Secure Zone', $e->getRealm() );
 			$this->assertEquals( 401, $e->getCode() );
-			throw $e;
+			return;
 		}
+		
+		$this->fail( 'Unauthorized exception was not thrown' );
 	}
📜 Review details

Configuration used: defaults

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 9198c85 and fb595d5.

📒 Files selected for processing (8)
  • src/Core/Exceptions/Forbidden.php
  • src/Core/Exceptions/Unauthorized.php
  • src/Core/ProblemDetails/ProblemDetails.php
  • src/Core/ProblemDetails/ProblemType.php
  • tests/Core/Exceptions/ForbiddenTest.php
  • tests/Core/Exceptions/UnauthorizedTest.php
  • tests/ProblemDetailsTest.php
  • versionlog.md
🧰 Additional context used
🧬 Code graph analysis (5)
src/Core/ProblemDetails/ProblemType.php (1)
src/Core/ProblemDetails/ProblemDetails.php (1)
  • ProblemDetails (52-239)
src/Core/Exceptions/Forbidden.php (2)
src/Core/Exceptions/Unauthorized.php (1)
  • __construct (39-47)
src/Core/ProblemDetails/ProblemDetails.php (1)
  • __construct (71-87)
tests/ProblemDetailsTest.php (2)
src/Core/ProblemDetails/ProblemDetails.php (9)
  • ProblemDetails (52-239)
  • getType (124-127)
  • getTitle (132-135)
  • getStatus (140-143)
  • getDetail (148-151)
  • getInstance (156-159)
  • getExtensions (164-167)
  • getExtension (175-178)
  • fromArray (225-238)
src/Core/ProblemDetails/ProblemType.php (3)
  • toProblemDetails (189-202)
  • getRecommendedStatus (139-155)
  • getDefaultTitle (162-179)
tests/Core/Exceptions/UnauthorizedTest.php (1)
src/Core/Exceptions/Unauthorized.php (2)
  • Unauthorized (29-58)
  • getRealm (54-57)
tests/Core/Exceptions/ForbiddenTest.php (1)
src/Core/Exceptions/Forbidden.php (3)
  • Forbidden (30-73)
  • getResource (59-62)
  • getPermission (69-72)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
  • GitHub Check: Cursor Bugbot
🔇 Additional comments (11)
src/Core/Exceptions/Forbidden.php (1)

1-73: LGTM!

Well-structured exception class with clear documentation. The implementation follows the same pattern as Unauthorized (from src/Core/Exceptions/Unauthorized.php), providing consistency across HTTP error exceptions. The nullable $resource and $permission properties offer flexibility for different usage scenarios.

src/Core/ProblemDetails/ProblemDetails.php (2)

94-119: LGTM! Comprehensive validation logic.

The validation correctly enforces RFC 9457 requirements:

  • Non-empty type and title
  • Status codes restricted to 4xx/5xx range
  • Extension keys follow identifier naming (compatible with both JSON and XML)
  • Reserved field names protected

225-238: LGTM!

The fromArray() factory method correctly extracts standard and extension fields. Empty defaults for type and title will appropriately trigger validation errors, ensuring malformed data is caught.

src/Core/Exceptions/Unauthorized.php (1)

1-58: LGTM!

Clean implementation consistent with the Forbidden exception pattern. The optional $realm property is useful for generating the WWW-Authenticate header in HTTP Basic Auth scenarios.

tests/Core/Exceptions/ForbiddenTest.php (1)

1-192: LGTM! Comprehensive test coverage.

The test suite thoroughly covers the Forbidden exception class including:

  • Constructor variations and default values
  • Getter return types and nullable behavior
  • Exception chaining
  • Inheritance verification
  • Edge cases (permission without resource)
tests/ProblemDetailsTest.php (1)

12-298: LGTM! Thorough test coverage for ProblemDetails and ProblemType.

The test suite comprehensively covers:

  • Creation and accessor methods
  • JSON serialization and array conversion
  • Reconstruction via fromArray()
  • All validation scenarios (empty fields, invalid status, extension constraints)
  • ProblemType enum integration including status codes and default titles
  • Complex nested extensions
src/Core/ProblemDetails/ProblemType.php (1)

1-203: LGTM! Well-designed enum with comprehensive coverage.

The ProblemType enum provides:

  • Clear documentation for each error type with expected HTTP status
  • Exhaustive match expressions in getRecommendedStatus() and getDefaultTitle()
  • Convenient toProblemDetails() factory method for quick ProblemDetails creation
  • Relative URI paths for portability across applications

The enum cases cover the most common HTTP error scenarios and align correctly with RFC 9457 recommendations.

tests/Core/Exceptions/UnauthorizedTest.php (4)

10-18: LGTM!

Default constructor test correctly validates all default values: message, code, realm, and previous exception.


20-65: Excellent constructor parameter coverage.

The test suite thoroughly validates all constructor parameter combinations including custom messages, realms, codes, and exception chaining. All assertions correctly match the expected behavior of the Unauthorized class.


67-82: LGTM!

Testing various realistic realm values (Basic Auth, Bearer Token, OAuth 2.0, etc.) provides good coverage for the realm functionality.


110-125: LGTM!

The inheritance and return type tests provide valuable validation of the exception's type hierarchy and the nullable return type behavior of getRealm().

Comment thread tests/ProblemDetailsTest.php
Comment thread versionlog.md
@codecov

codecov Bot commented Jan 7, 2026

Copy link
Copy Markdown

Welcome to Codecov 🎉

Once you merge this PR into your default branch, you're all set! Codecov will compare coverage reports and display results in all future pull requests.

ℹ️ You can also turn on project coverage checks and project coverage reporting on Pull Request comment

Thanks for integrating Codecov - We've got you covered ☂️

Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
@ljonesfl ljonesfl closed this Jan 7, 2026
@ljonesfl
ljonesfl deleted the feature/problem-details branch January 7, 2026 21:35
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