refactor: replace manual Response(4xx/5xx) with HttpResponseException - #220
Merged
Conversation
Manual `Response(4xx)` and `Response(5xx)` returns across event namespaces are replaced with throwing `HttpResponseException` from `package:google_cloud_shelf`. This ensures that `createLoggingMiddleware` records warning and error logs with full stack traces, inner errors, and trace correlation in Cloud Logging. Summary of Changes: - Replace manual `Response(400)` and `Response(500)` returns across 13 namespace files (`alerts`, `app_distribution`, `billing`, `crashlytics`, `performance`, `database`, `eventarc`, `firestore`, `pubsub`, `remote_config`, `storage`, `tasks`, `test_lab`) with throwing `HttpResponseException`. - Replace manual 501 responses in `firestore_namespace.dart` with throwing `HttpResponseException.notImplemented`. - Update 400 error unit tests in `storage_test.dart` and `remote_config_test.dart` to use `findHandler` for testing via the full middleware pipeline. Fixes #215
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors error handling across multiple namespace files by replacing direct Response returns with throwing HttpResponseException (such as badRequest, notImplemented, and internalServerError) from the google_cloud_shelf package. It also updates catch blocks to capture and propagate stack traces and inner errors, and updates unit tests to use findHandler instead of _findFunction. There are no review comments to address, and I have no additional feedback to provide.
brianquinlan
approved these changes
Jul 7, 2026
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.
Manual
Response(4xx)andResponse(5xx)returns across event namespaces are replaced with throwingHttpResponseExceptionfrompackage:google_cloud_shelf. This ensures thatcreateLoggingMiddlewarerecords warning and error logs with full stack traces, inner errors, and trace correlation in Cloud Logging.Summary of Changes:
Response(400)andResponse(500)returns across 13 namespace files (alerts,app_distribution,billing,crashlytics,performance,database,eventarc,firestore,pubsub,remote_config,storage,tasks,test_lab) with throwingHttpResponseException.firestore_namespace.dartwith throwingHttpResponseException.notImplemented.storage_test.dartandremote_config_test.dartto usefindHandlerfor testing via the full middleware pipeline.Fixes #215