refactor: introduce HttpResponseException and update factories#227
refactor: introduce HttpResponseException and update factories#227kevmoo wants to merge 11 commits intogoogleapis:mainfrom
Conversation
- Rename `bad_request_exception.dart` to `http_response_exception.dart` and introduce `HttpResponseException` to replace `BadRequestException` (now a typedef alias for backward compatibility). - Deprecate `badRequestMiddleware` in favor of `httpResponseExceptionMiddleware`. - Make `message` a named parameter in all `HttpResponseException` factory constructors. - Provide default HTTP status messages (e.g., 'Bad Request', 'Not Found') if `message` is null. - Update callers in `example/example.dart` and `test/logging_test.dart` to use named arguments or remove redundant ones. - Document default behavior in doc comments using standard Dart style. - Update `CHANGELOG.md` to use precise "Introduces" language.
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces HttpResponseException and httpResponseExceptionMiddleware, deprecating the previous BadRequestException and badRequestMiddleware. The new exception class expands support to include 5XX status codes and provides factory constructors for common HTTP errors with default messages and gRPC-aligned status strings. Logging middleware has been updated to handle these exceptions and include structured error payloads in Google Cloud logs. Feedback includes fixing a typo in the changelog, updating documentation to reflect the expanded status code range, and adding missing default status values for the new 5XX factory constructors to ensure consistency with AIP-193.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces HttpResponseException and httpResponseExceptionMiddleware, deprecating BadRequestException and its associated middleware. The new exception class expands support to HTTP 5XX status codes and structured error reporting. Review feedback identifies that changing factory constructor parameters from positional to named is a breaking change, which contradicts the goal of backward compatibility. Additionally, it is recommended to update the logging logic to use LogSeverity.error for 5XX status codes instead of a hardcoded warning level.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces HttpResponseException and httpResponseExceptionMiddleware, deprecating the BadRequestException equivalents while adding support for 5XX status codes and structured error reporting. It also includes a new utility for header-based JSON response negotiation. Feedback points out that changing factory constructor parameters from positional to named is a breaking change that may require a version bump to 0.5.0. Additionally, the mime package was added as a dependency but remains unused in the implementation.
|
@brianquinlan – I'd love to publish this tomorrow (Friday) – final thoughts? |
2 participants
bad_request_exception.darttohttp_response_exception.dartand introduceHttpResponseExceptionto replaceBadRequestException(now a typedef alias for backward compatibility).badRequestMiddlewarein favor ofhttpResponseExceptionMiddleware.messagea named parameter in allHttpResponseExceptionfactory constructors.messageis null.example/example.dartandtest/logging_test.dartto use named arguments or remove redundant ones.CHANGELOG.mdto use precise "Introduces" language.