Merge pull request #48 from github/error-responses

Consistent Error Responses

Author: GrantBirki

Gemfile

@@ -6,6 +6,7 @@ gemspec
 
 group :development do
   gem "irb", "~> 1"
+  gem "ostruct", "~> 0.6.1"
   gem "rack-test", "~> 2.2"
   gem "rspec", "~> 3"
   gem "rubocop", "~> 1"

Gemfile.lock

@@ -95,6 +95,7 @@ GEM
     net-http (0.6.0)
       uri
     nio4r (2.7.4)
+    ostruct (0.6.1)
     parallel (1.27.0)
     parser (3.3.8.0)
       ast (~> 2.4.1)
@@ -221,6 +222,7 @@ PLATFORMS
 DEPENDENCIES
   hooks-ruby!
   irb (~> 1)
+  ostruct (~> 0.6.1)
   rack-test (~> 2.2)
   rspec (~> 3)
   rubocop (~> 1)

README.md

@@ -58,7 +58,7 @@ Here is a very high-level overview of how Hooks works:
     ```ruby
     # file: plugins/handlers/my_custom_handler.rb
     class MyCustomHandler < Hooks::Plugins::Handlers::Base
-      def call(payload:, headers:, config:)
+      def call(payload:, headers:, env:, config:)
         # Process the incoming webhook - optionally use the payload and headers
         # to perform some action or validation
         # For this example, we will just return a success message
@@ -233,7 +233,7 @@ Create custom handler plugins in the `plugins/handlers` directory to process inc
 ```ruby
 # file: plugins/handlers/hello_handler.rb
 class HelloHandler < Hooks::Plugins::Handlers::Base
-  def call(payload:, headers:, config:)
+  def call(payload:, headers:, env:, config:)
     # Process the incoming webhook - optionally use the payload and headers
     # to perform some action or validation
     # For this example, we will just return a success message
@@ -251,7 +251,7 @@ And another handler plugin for the `/goodbye` endpoint:
 ```ruby
 # file: plugins/handlers/goodbye_handler.rb
 class GoodbyeHandler < Hooks::Plugins::Handlers::Base
-  def call(payload:, headers:, config:)
+  def call(payload:, headers:, env:, config:)
     # Ditto for the goodbye endpoint
     {
       message: "goodbye webhook processed successfully",

docs/design.md

@@ -33,7 +33,7 @@ Note: The `hooks` gem name is already taken on RubyGems, so this project is name
 2. **Plugin Architecture**
 
    * **Team Handlers**: `class MyHandler < Hooks::Plugins::Handlers::Base`
-     * Must implement `#call(payload:, headers:, config:)` method
+     * Must implement `#call(payload:, headers:, env:, config:)` method
      * `payload`: parsed request body (JSON Hash or raw String)
      * `headers`: HTTP headers as Hash with string keys
      * `config`: merged endpoint configuration including `opts` section
@@ -230,7 +230,7 @@ endpoints_dir:   ./config/endpoints # directory containing endpoint configs
        * **Before**: enforce `request_limit`, `request_timeout`
        * **Signature**: call custom or default validator
        * **Hooks**: run `on_request` plugins
-     * **Handler**: invoke `MyHandler.new.call(payload:, headers:, config:)`
+     * **Handler**: invoke `MyHandler.new.call(payload:, headers:, env:, config:)`
      * **After**: run `on_response` plugins
      * **Rescue**: on exception, run `on_error`, rethrow or format JSON error
 
@@ -528,9 +528,10 @@ Base class for all webhook handlers.
 class MyHandler < Hooks::Plugins::Handlers::Base
   # @param payload [Hash, String] Parsed request body or raw string
   # @param headers [Hash<String, String>] HTTP headers
+  # @param env [Hash] Rack environment (includes request context)
   # @param config [Hash] Merged endpoint configuration
   # @return [Hash, String, nil] Response body (auto-converted to JSON)
-  def call(payload:, headers:, config:)
+  def call(payload:, headers:, env:, config:)
     # Handler implementation
     { status: "processed", id: generate_id }
   end

docs/handler_plugins.md

@@ -17,9 +17,10 @@ class Example < Hooks::Plugins::Handlers::Base
   #
   # @param payload [Hash, String] webhook payload (pure JSON with string keys)
   # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized)
+  # @param env [Hash] A modifed Rack environment that contains a lot of context about the request
   # @param config [Hash] Endpoint configuration
   # @return [Hash] Response data
-  def call(payload:, headers:, config:)
+  def call(payload:, headers:, env:, config:)
     return {
       status: "success"
     }
@@ -99,6 +100,34 @@ It should be noted that the `headers` parameter is a Hash with **string keys** (
 
 You can disable header normalization by either setting the environment variable `HOOKS_NORMALIZE_HEADERS` to `false` or by setting the `normalize_headers` option to `false` in the global configuration file.
 
+### `env` Parameter
+
+The `env` parameter is a Hash that contains a modified Rack environment. It provides a lot of context about the request, including information about the request method, path, query parameters, and more. This can be useful for debugging or for accessing additional request information. It is considered *everything plus the kitchen sink* that you might need to know about the request.
+
+Here is a partial example of what the `env` parameter might look like:
+
+```ruby
+{
+  "REQUEST_METHOD" => "POST",
+  "PATH_INFO" => "/webhooks/example",
+  "QUERY_STRING" => "foo=bar&baz=123",
+  "HTTP_VERSION" => "HTTP/1.1",
+  "REQUEST_URI" => "https://hooks.example.com/webhooks/example?foo=bar&baz=qux",
+  "SERVER_NAME" => "hooks.example.com",
+  "SERVER_PORT" => 443,
+  "CONTENT_TYPE" => "application/json",
+  "CONTENT_LENGTH" => 123,
+  "REMOTE_ADDR" => "<IP_ADDRESS>",
+  "hooks.request_id" => "<REQUEST_ID>",
+  "hooks.handler" => "ExampleHandler"
+  "hooks.endpoint_config" => {}
+  "hooks.start_time" => "2023-10-01T12:34:56Z",
+  # etc...
+}
+```
+
+For the complete list of available keys in the `env` parameter, you can refer to the source code at [`lib/hooks/app/rack_env_builder.rb`](../lib/hooks/app/rack_env_builder.rb).
+
 ### `config` Parameter
 
 The `config` parameter is a Hash (symbolized) that contains the endpoint configuration. This can include any additional settings or parameters that you want to use in your handler. Most of the time, this won't be used, but sometimes endpoint configs add `opts` that can be useful for the handler.
@@ -123,9 +152,10 @@ class Example < Hooks::Plugins::Handlers::Base
   #
   # @param payload [Hash, String] Webhook payload
   # @param headers [Hash<String, String>] HTTP headers
+  # @param env [Hash] A modified Rack environment that contains a lot of context about the request
   # @param config [Hash] Endpoint configuration
   # @return [Hash] Response data
-  def call(payload:, headers:, config:)
+  def call(payload:, headers:, env:, config:)
     result = Retryable.with_context(:default) do
       some_operation_that_might_fail()
     end

docs/instrument_plugins.md

@@ -160,7 +160,7 @@ Once configured, your custom instruments are available throughout the applicatio
 
 ```ruby
 class MyHandler < Hooks::Plugins::Handlers::Base
-  def call(payload:, headers:, config:)
+  def call(payload:, headers:, env:, config:)
     # Use your custom stats methods
     stats.increment("handler.calls", { handler: "MyHandler" })
 

lib/hooks/app/api.rb

@@ -5,6 +5,7 @@
 require "securerandom"
 require_relative "helpers"
 require_relative "auth/auth"
+require_relative "rack_env_builder"
 require_relative "../plugins/handlers/base"
 require_relative "../plugins/handlers/default"
 require_relative "../core/logger_factory"
@@ -65,41 +66,27 @@ def self.create(config:, endpoints:, log:)
               Core::LogContext.with(request_context) do
                 begin
                   # Build Rack environment for lifecycle hooks
-                  rack_env = {
-                    "REQUEST_METHOD" => request.request_method,
-                    "PATH_INFO" => request.path_info,
-                    "QUERY_STRING" => request.query_string,
-                    "HTTP_VERSION" => request.env["HTTP_VERSION"],
-                    "REQUEST_URI" => request.url,
-                    "SERVER_NAME" => request.env["SERVER_NAME"],
-                    "SERVER_PORT" => request.env["SERVER_PORT"],
-                    "CONTENT_TYPE" => request.content_type,
-                    "CONTENT_LENGTH" => request.content_length,
-                    "REMOTE_ADDR" => request.env["REMOTE_ADDR"],
-                    "hooks.request_id" => request_id,
-                    "hooks.handler" => handler_class_name,
-                    "hooks.endpoint_config" => endpoint_config,
-                    "hooks.start_time" => start_time.iso8601,
-                    "hooks.full_path" => full_path
-                  }
-
-                  # Add HTTP headers to environment
-                  headers.each do |key, value|
-                    env_key = "HTTP_#{key.upcase.tr('-', '_')}"
-                    rack_env[env_key] = value
-                  end
+                  rack_env_builder = RackEnvBuilder.new(
+                    request,
+                    headers,
+                    request_context,
+                    endpoint_config,
+                    start_time,
+                    full_path
+                  )
+                  rack_env = rack_env_builder.build
 
                   # Call lifecycle hooks: on_request
                   Core::PluginLoader.lifecycle_plugins.each do |plugin|
                     plugin.on_request(rack_env)
                   end
 
-                  enforce_request_limits(config)
+                  enforce_request_limits(config, request_context)
                   request.body.rewind
                   raw_body = request.body.read
 
                   if endpoint_config[:auth]
-                    validate_auth!(raw_body, headers, endpoint_config, config)
+                    validate_auth!(raw_body, headers, endpoint_config, config, request_context)
                   end
 
                   payload = parse_payload(raw_body, headers, symbolize: false)
@@ -109,6 +96,7 @@ def self.create(config:, endpoints:, log:)
                   response = handler.call(
                     payload:,
                     headers: processed_headers,
+                    env: rack_env,
                     config: endpoint_config
                   )
 
@@ -123,21 +111,32 @@ def self.create(config:, endpoints:, log:)
                   content_type "application/json"
                   response.to_json
                 rescue StandardError => e
-                  # Call lifecycle hooks: on_error
+                  err_msg = "Error processing webhook event with handler: #{handler_class_name} - #{e.message} " \
+                    "- request_id: #{request_id} - path: #{full_path} - method: #{http_method} - " \
+                    "backtrace: #{e.backtrace.join("\n")}"
+                  log.error(err_msg)
+
+                  # call lifecycle hooks: on_error if the rack_env is available
+                  # if the rack_env is not available, it means the error occurred before we could build it
                   if defined?(rack_env)
                     Core::PluginLoader.lifecycle_plugins.each do |plugin|
                       plugin.on_error(e, rack_env)
                     end
                   end
 
-                  log.error("an error occuring during the processing of a webhook event - #{e.message}")
+                  # construct a standardized error response
                   error_response = {
-                    error: e.message,
-                    code: determine_error_code(e),
+                    error: "server_error",
+                    message: "an unexpected error occurred while processing the request",
                     request_id:
                   }
-                  error_response[:backtrace] = e.backtrace unless config[:production]
-                  status error_response[:code]
+
+                  # enrich the error response with details if not in production
+                  error_response[:backtrace] = e.backtrace.join("\n") unless config[:production]
+                  error_response[:message] = e.message unless config[:production]
+                  error_response[:handler] = handler_class_name unless config[:production]
+
+                  status determine_error_code(e)
                   content_type "application/json"
                   error_response.to_json
                 end

lib/hooks/app/auth/auth.rb

@@ -16,30 +16,45 @@ module Auth
       # @param headers [Hash] The request headers.
       # @param endpoint_config [Hash] The endpoint configuration, must include :auth key.
       # @param global_config [Hash] The global configuration (optional, for compatibility).
+      # @param request_context [Hash] Context for the request, e.g. request ID, path, handler (optional).
       # @raise [StandardError] Raises error if authentication fails or is misconfigured.
       # @return [void]
       # @note This method will halt execution with an error if authentication fails.
-      def validate_auth!(payload, headers, endpoint_config, global_config = {})
+      def validate_auth!(payload, headers, endpoint_config, global_config = {}, request_context = {})
         auth_config = endpoint_config[:auth]
+        request_id = request_context&.dig(:request_id)
 
         # Ensure auth type is present and valid
         auth_type = auth_config&.dig(:type)
         unless auth_type&.is_a?(String) && !auth_type.strip.empty?
-          error!("authentication configuration missing or invalid", 500)
+          log.error("authentication configuration missing or invalid - request_id: #{request_id}")
+          error!({
+            error: "authentication_configuration_error",
+            message: "authentication configuration missing or invalid",
+            request_id:
+          }, 500)
         end
 
         # Get auth plugin from loaded plugins registry (boot-time loaded only)
         begin
           auth_class = Core::PluginLoader.get_auth_plugin(auth_type)
         rescue => e
-          log.error("failed to load auth plugin '#{auth_type}': #{e.message}")
-          error!("unsupported auth type '#{auth_type}'", 400)
+          log.error("failed to load auth plugin '#{auth_type}': #{e.message} - request_id: #{request_id}")
+          error!({
+            error: "authentication_plugin_error",
+            message: "unsupported auth type '#{auth_type}'",
+            request_id:
+          }, 400)
         end
 
         log.debug("validating auth for request with auth_class: #{auth_class.name}")
         unless auth_class.valid?(payload:, headers:, config: endpoint_config)
-          log.warn("authentication failed for request with auth_class: #{auth_class.name}")
-          error!("authentication failed", 401)
+          log.warn("authentication failed for request with auth_class: #{auth_class.name} - request_id: #{request_id}")
+          error!({
+            error: "authentication_failed",
+            message: "authentication failed",
+            request_id:
+          }, 401)
         end
       end
 

lib/hooks/app/endpoints/catchall.rb

@@ -27,20 +27,36 @@ def self.route_block(captured_config, captured_logger)
         # :nocov:
         proc do
           request_id = uuid
+          start_time = Time.now
 
           # Use captured values
           config = captured_config
           log = captured_logger
 
+          full_path = "#{config[:root_path]}/#{params[:path]}"
+
+          handler_class_name = "DefaultHandler"
+          http_method = "post"
+
           # Set request context for logging
           request_context = {
             request_id:,
-            path: "/#{params[:path]}",
-            handler: "DefaultHandler"
+            path: full_path,
+            handler: handler_class_name
           }
 
           Hooks::Core::LogContext.with(request_context) do
             begin
+              rack_env_builder = RackEnvBuilder.new(
+                request,
+                headers,
+                request_context,
+                config,
+                start_time,
+                full_path
+              )
+              rack_env = rack_env_builder.build
+
               # Enforce request limits
               enforce_request_limits(config)
 
@@ -58,32 +74,34 @@ def self.route_block(captured_config, captured_logger)
               response = handler.call(
                 payload:,
                 headers:,
+                env: rack_env,
                 config: {}
               )
 
-              log.info "request processed successfully with default handler (id: #{request_id})"
-
-              # Return response as JSON string when using txt format
+              log.info("successfully processed webhook event with handler: #{handler_class_name}")
+              log.debug("processing duration: #{Time.now - start_time}s")
               status 200
               content_type "application/json"
-              (response || { status: "ok" }).to_json
-
+              response.to_json
             rescue StandardError => e
-              log.error "request failed: #{e.message} (id: #{request_id})"
+              err_msg = "Error processing webhook event with handler: #{handler_class_name} - #{e.message} " \
+                "- request_id: #{request_id} - path: #{full_path} - method: #{http_method} - " \
+                "backtrace: #{e.backtrace.join("\n")}"
+              log.error(err_msg)
 
-              # Return error response
+              # construct a standardized error response
               error_response = {
-                error: e.message,
-                code: determine_error_code(e),
-                request_id: request_id
+                error: "server_error",
+                message: "an unexpected error occurred while processing the request",
+                request_id:
               }
 
-              # Add backtrace in all environments except production
-              unless config[:production] == true
-                error_response[:backtrace] = e.backtrace
-              end
+              # enrich the error response with details if not in production
+              error_response[:backtrace] = e.backtrace.join("\n") unless config[:production]
+              error_response[:message] = e.message unless config[:production]
+              error_response[:handler] = handler_class_name unless config[:production]
 
-              status error_response[:code]
+              status determine_error_code(e)
               content_type "application/json"
               error_response.to_json
             end