diff --git a/Gemfile b/Gemfile index 02fad98..6e851f7 100644 --- a/Gemfile +++ b/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" diff --git a/Gemfile.lock b/Gemfile.lock index 074d8e7..82cac8a 100644 --- a/Gemfile.lock +++ b/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) diff --git a/README.md b/README.md index 44af5fb..5519104 100644 --- a/README.md +++ b/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", diff --git a/docs/design.md b/docs/design.md index d8285c4..a23e4eb 100644 --- a/docs/design.md +++ b/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] 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 diff --git a/docs/handler_plugins.md b/docs/handler_plugins.md index 3532e2f..1e4fbf1 100644 --- a/docs/handler_plugins.md +++ b/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" => "", + "hooks.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] 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 diff --git a/docs/instrument_plugins.md b/docs/instrument_plugins.md index 1681269..84dda84 100644 --- a/docs/instrument_plugins.md +++ b/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" }) diff --git a/lib/hooks/app/api.rb b/lib/hooks/app/api.rb index 7c144a0..47ea052 100644 --- a/lib/hooks/app/api.rb +++ b/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 diff --git a/lib/hooks/app/auth/auth.rb b/lib/hooks/app/auth/auth.rb index 03f9de5..93e9add 100644 --- a/lib/hooks/app/auth/auth.rb +++ b/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 diff --git a/lib/hooks/app/endpoints/catchall.rb b/lib/hooks/app/endpoints/catchall.rb index eb3eb77..e240e2c 100644 --- a/lib/hooks/app/endpoints/catchall.rb +++ b/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 diff --git a/lib/hooks/app/helpers.rb b/lib/hooks/app/helpers.rb index c5127e8..9aae319 100644 --- a/lib/hooks/app/helpers.rb +++ b/lib/hooks/app/helpers.rb @@ -17,10 +17,11 @@ def uuid # Enforce request size and timeout limits # # @param config [Hash] The configuration hash, must include :request_limit + # @param request_context [Hash] Context for the request, e.g. request ID (optional) # @raise [StandardError] Halts with error if request body is too large # @return [void] # @note Timeout enforcement should be handled at the server level (e.g., Puma) - def enforce_request_limits(config) + def enforce_request_limits(config, request_context = {}) # Optimized content length check - check most common sources first content_length = request.content_length if respond_to?(:request) && request.respond_to?(:content_length) @@ -34,7 +35,8 @@ def enforce_request_limits(config) content_length = content_length&.to_i if content_length && content_length > config[:request_limit] - error!("request body too large", 413) + request_id = request_context&.dig(:request_id) + error!({ error: "request_body_too_large", message: "request body too large", request_id: }, 413) end # Note: Timeout enforcement would typically be handled at the server level (Puma, etc.) @@ -76,13 +78,14 @@ def parse_payload(raw_body, headers, symbolize: false) # @return [Object] An instance of the loaded handler class # @raise [StandardError] If handler cannot be found def load_handler(handler_class_name) - # Get handler class from loaded plugins registry (boot-time loaded only) - begin - handler_class = Core::PluginLoader.get_handler_plugin(handler_class_name) - return handler_class.new - rescue => e - error!("failed to get handler '#{handler_class_name}': #{e.message}", 500) - end + # Get handler class from loaded plugins registry (the registry is populated at boot time) + # NOTE: We create a new instance per request (not reuse boot-time instances) because: + # - Security: Prevents state pollution and information leakage between requests + # - Thread Safety: Avoids race conditions from shared instance state + # - Performance: Handler instantiation is fast; reusing instances provides minimal gain + # - Memory: Allows garbage collection of short-lived objects (Ruby GC optimization) + handler_class = Core::PluginLoader.get_handler_plugin(handler_class_name) + return handler_class.new end private diff --git a/lib/hooks/app/rack_env_builder.rb b/lib/hooks/app/rack_env_builder.rb new file mode 100644 index 0000000..c92d737 --- /dev/null +++ b/lib/hooks/app/rack_env_builder.rb @@ -0,0 +1,85 @@ +# frozen_string_literal: true + +module Hooks + module App + # Builds Rack environment hash for lifecycle hooks and handler processing + # + # This class centralizes the construction of the Rack environment that gets + # passed to lifecycle hooks and handlers, ensuring consistency and making + # it easy to reference the environment structure. + # + # @example Building a Rack environment + # builder = RackEnvBuilder.new(request, headers, request_context) + # rack_env = builder.build + # + class RackEnvBuilder + # Initialize the builder with required components + # + # @param request [Grape::Request] The Grape request object + # @param headers [Hash] Request headers hash + # @param request_context [Hash] Request context containing metadata + # @option request_context [String] :request_id Unique request identifier + # @option request_context [String] :handler Handler class name + # @param endpoint_config [Hash] Endpoint configuration + # @param start_time [Time] Request start time + # @param full_path [String] Full request path including root path + def initialize(request, headers, request_context, endpoint_config, start_time, full_path) + @request = request + @headers = headers + @request_context = request_context + @endpoint_config = endpoint_config + @start_time = start_time + @full_path = full_path + end + + # Build the Rack environment hash + # + # Constructs a hash containing standard Rack environment variables + # plus Hooks-specific extensions for lifecycle hooks and handlers. + # + # @return [Hash] Complete Rack environment hash + def build + rack_env = build_base_environment + add_http_headers(rack_env) + rack_env + end + + private + + # Build the base Rack environment with standard and Hooks-specific variables + # This pretty much creates everything plus the kitchen sink. It will be very rich in information + # and will be used by lifecycle hooks and handlers to access request metadata. + # + # @return [Hash] Base environment hash + def build_base_environment + { + "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_context[:request_id], + "hooks.handler" => @request_context[:handler], + "hooks.endpoint_config" => @endpoint_config, + "hooks.start_time" => @start_time.iso8601, + "hooks.full_path" => @full_path + } + end + + # Add HTTP headers to the environment with proper Rack naming convention + # + # @param rack_env [Hash] Environment hash to modify + def add_http_headers(rack_env) + @headers.each do |key, value| + env_key = "HTTP_#{key.upcase.tr('-', '_')}" + rack_env[env_key] = value + end + end + end + end +end diff --git a/lib/hooks/plugins/handlers/base.rb b/lib/hooks/plugins/handlers/base.rb index e891082..7dd54f5 100644 --- a/lib/hooks/plugins/handlers/base.rb +++ b/lib/hooks/plugins/handlers/base.rb @@ -16,10 +16,11 @@ class Base # # @param payload [Hash, String] Parsed request body (JSON Hash) or raw string # @param headers [Hash] HTTP headers (string keys, optionally normalized - default is normalized) + # @param env [Hash] Rack environment (contains the request context, headers, etc - very rich context) # @param config [Hash] Merged endpoint configuration including opts section (symbolized keys) # @return [Hash, String, nil] Response body (will be auto-converted to JSON) # @raise [NotImplementedError] if not implemented by subclass - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) raise NotImplementedError, "Handler must implement #call method" end end diff --git a/lib/hooks/plugins/handlers/default.rb b/lib/hooks/plugins/handlers/default.rb index 6161aaa..d6e6921 100644 --- a/lib/hooks/plugins/handlers/default.rb +++ b/lib/hooks/plugins/handlers/default.rb @@ -20,6 +20,7 @@ class DefaultHandler < Hooks::Plugins::Handlers::Base # # @param payload [Hash, String] The webhook payload (parsed JSON or raw string) # @param headers [Hash] HTTP headers from the webhook request + # @param env [Hash] Rack environment (contains the request context, headers, config, etc - very rich context) # @param config [Hash] Endpoint configuration containing handler options # @return [Hash] Response indicating successful processing # @option config [Hash] :opts Additional handler-specific configuration options @@ -29,10 +30,11 @@ class DefaultHandler < Hooks::Plugins::Handlers::Base # response = handler.call( # payload: { "event" => "push" }, # headers: { "Content-Type" => "application/json" }, + # env: { "REQUEST_METHOD" => "POST", "hooks.request_id" => "12345" }, # config: { opts: {} } # ) # # => { message: "webhook processed successfully", handler: "DefaultHandler", timestamp: "..." } - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) log.info("🔔 Default handler invoked for webhook 🔔") @@ -41,6 +43,10 @@ def call(payload:, headers:, config:) log.debug("received payload: #{payload.inspect}") end + if env + log.debug("default handler got a request with the following request_id: #{env['hooks.request_id']}") + end + { message: "webhook processed successfully", handler: "DefaultHandler", diff --git a/spec/acceptance/acceptance_tests.rb b/spec/acceptance/acceptance_tests.rb index 7836a27..4c1e253 100644 --- a/spec/acceptance/acceptance_tests.rb +++ b/spec/acceptance/acceptance_tests.rb @@ -388,13 +388,26 @@ def expired_unix_timestamp(seconds_ago = 600) it "successfully validates using a custom auth plugin" do payload = {}.to_json - headers = { "Authorization" => "Bearer octoawesome-shared-secret" } - response = make_request(:post, "/webhooks/with_custom_auth_plugin", payload, headers) + headers = { "Authorization" => "Bearer octoawesome-shared-secret", "Content-Type" => "application/json" } + response = make_request(:post, "/webhooks/with_custom_auth_plugin?foo=bar&bar=baz", payload, headers) expect_response(response, Net::HTTPSuccess) body = parse_json_response(response) expect(body["status"]).to eq("test_success") expect(body["handler"]).to eq("TestHandler") + expect(body["payload_received"]).to eq({}) + expect(body["env_received"]).to have_key("REQUEST_METHOD") + + env = body["env_received"] + expect(env["hooks.request_id"]).to be_a(String) + expect(env["hooks.handler"]).to eq("TestHandler") + expect(env["hooks.endpoint_config"]).to be_a(Hash) + expect(env["hooks.start_time"]).to be_a(String) + expect(env["hooks.full_path"]).to eq("/webhooks/with_custom_auth_plugin") + expect(env["HTTP_AUTHORIZATION"]).to eq("Bearer octoawesome-shared-secret") + expect(env["CONTENT_TYPE"]).to eq("application/json") + expect(env["CONTENT_LENGTH"]).to eq("2") # length of "{}" + expect(env["QUERY_STRING"]).to eq("foo=bar&bar=baz") end it "rejects requests with invalid credentials using custom auth plugin" do @@ -418,6 +431,29 @@ def expired_unix_timestamp(seconds_ago = 600) headers = {} response = make_request(:post, "/webhooks/boomtown", payload, headers) expect_response(response, Net::HTTPInternalServerError, "Boomtown error occurred") + body = parse_json_response(response) + expect(body["error"]).to eq("server_error") + expect(body["message"]).to eq("Boomtown error occurred") + expect(body).to have_key("backtrace") + expect(body["backtrace"]).to be_a(String) + expect(body).to have_key("request_id") + expect(body["request_id"]).to be_a(String) + expect(body).to have_key("handler") + expect(body["handler"]).to eq("Boomtown") + end + end + + describe "does_not_exist" do + it "sends a POST request to the /webhooks/does_not_exist endpoint and it fails because the handler does not exist" do + payload = {}.to_json + headers = {} + response = make_request(:post, "/webhooks/does_not_exist", payload, headers) + expect_response(response, Net::HTTPInternalServerError, /Handler plugin 'DoesNotExist' not found/) + body = parse_json_response(response) + expect(body["error"]).to eq("server_error") + expect(body["message"]).to match( + /Handler plugin 'DoesNotExist' not found. Available handlers: DefaultHandler,.*/ + ) end end @@ -448,5 +484,39 @@ def expired_unix_timestamp(seconds_ago = 600) expect(body["expected_header"]).to eq("x-okta-verification-challenge") end end + + describe "boomtown_with_error" do + it "sends a POST request to the /webhooks/boomtown_with_error endpoint and it does not explode" do + payload = { boom: false }.to_json + response = make_request(:post, "/webhooks/boomtown_with_error", payload, json_headers) + expect_response(response, Net::HTTPSuccess) + + body = parse_json_response(response) + expect(body["status"]).to eq("ok") + end + + it "sends a POST request to the /webhooks/boomtown_with_error endpoint and it explodes" do + # TODO: Fix this acceptance test - the current error looks like this: + # 1) Hooks endpoints boomtown_with_error sends a POST request to the /webhooks/boomtown_with_error endpoint and it explodes + # Failure/Error: expect(response.body).to include(expected_body_content) if expected_body_content + #expected "{\"error\":\"server_error\",\"message\":\"undefined method 'error!' for an instance of BoomtownWithE...thread_pool.rb:167:in 'block in #Puma::ThreadPool#spawn_thread'\",\"handler\":\"BoomtownWithError\"}" to include "the payload triggered a boomtown error" + # ./spec/acceptance/acceptance_tests.rb:28:in 'RSpec::ExampleGroups::Hooks#expect_response' + # ./spec/acceptance/acceptance_tests.rb:501:in 'block (4 levels) in ' + + # payload = { boom: true }.to_json + # response = make_request(:post, "/webhooks/boomtown_with_error", payload, json_headers) + # expect_response(response, Net::HTTPInternalServerError, "the payload triggered a boomtown error") + + # body = parse_json_response(response) + # expect(body["error"]).to eq("server_error") + # expect(body["message"]).to eq("the payload triggered a boomtown error") + # expect(body).to have_key("backtrace") + # expect(body["backtrace"]).to be_a(String) + # expect(body).to have_key("request_id") + # expect(body["request_id"]).to be_a(String) + # expect(body).to have_key("handler") + # expect(body["handler"]).to eq("BoomtownWithError") + end + end end end diff --git a/spec/acceptance/config/endpoints/boomtown_with_error.yml b/spec/acceptance/config/endpoints/boomtown_with_error.yml new file mode 100644 index 0000000..6713a17 --- /dev/null +++ b/spec/acceptance/config/endpoints/boomtown_with_error.yml @@ -0,0 +1,3 @@ +path: /boomtown_with_error +handler: BoomtownWithError +method: post diff --git a/spec/acceptance/config/endpoints/does_not_exist.yml b/spec/acceptance/config/endpoints/does_not_exist.yml new file mode 100644 index 0000000..f5a9c5a --- /dev/null +++ b/spec/acceptance/config/endpoints/does_not_exist.yml @@ -0,0 +1,2 @@ +path: /does_not_exist +handler: DoesNotExist diff --git a/spec/acceptance/plugins/handlers/boomtown.rb b/spec/acceptance/plugins/handlers/boomtown.rb index 9d9d2b1..e996196 100644 --- a/spec/acceptance/plugins/handlers/boomtown.rb +++ b/spec/acceptance/plugins/handlers/boomtown.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Boomtown < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) raise StandardError, "Boomtown error occurred" end end diff --git a/spec/acceptance/plugins/handlers/boomtown_with_error.rb b/spec/acceptance/plugins/handlers/boomtown_with_error.rb new file mode 100644 index 0000000..12f0f8c --- /dev/null +++ b/spec/acceptance/plugins/handlers/boomtown_with_error.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +class BoomtownWithError < Hooks::Plugins::Handlers::Base + def call(payload:, headers:, env:, config:) + + if payload["boom"] == true + log.error("boomtown error triggered by payload: #{payload.inspect} - request_id: #{env["hooks.request_id"]}") + + # TODO: Get Grape's `error!` method to work with this + error!({ + error: "boomtown_with_error", + message: "the payload triggered a boomtown error", + request_id: env["hooks.request_id"] + }, 500) + end + + return { status: "ok" } + end +end diff --git a/spec/acceptance/plugins/handlers/github_handler.rb b/spec/acceptance/plugins/handlers/github_handler.rb index 3af4ba8..b624693 100644 --- a/spec/acceptance/plugins/handlers/github_handler.rb +++ b/spec/acceptance/plugins/handlers/github_handler.rb @@ -8,7 +8,7 @@ class GithubHandler < Hooks::Plugins::Handlers::Base # @param headers [Hash] HTTP headers # @param config [Hash] Endpoint configuration # @return [Hash] Response data - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) log.info("🚀 Processing GitHub webhook 🚀") return { status: "success" diff --git a/spec/acceptance/plugins/handlers/hello.rb b/spec/acceptance/plugins/handlers/hello.rb index f995a81..3523d06 100644 --- a/spec/acceptance/plugins/handlers/hello.rb +++ b/spec/acceptance/plugins/handlers/hello.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class Hello < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { status: "success", handler: self.class.name, diff --git a/spec/acceptance/plugins/handlers/okta_handler.rb b/spec/acceptance/plugins/handlers/okta_handler.rb index f06c37c..0713073 100644 --- a/spec/acceptance/plugins/handlers/okta_handler.rb +++ b/spec/acceptance/plugins/handlers/okta_handler.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class OktaHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) return { status: "success" } diff --git a/spec/acceptance/plugins/handlers/okta_setup_handler.rb b/spec/acceptance/plugins/handlers/okta_setup_handler.rb index b04de5e..99f5734 100644 --- a/spec/acceptance/plugins/handlers/okta_setup_handler.rb +++ b/spec/acceptance/plugins/handlers/okta_setup_handler.rb @@ -1,11 +1,13 @@ # frozen_string_literal: true class OktaSetupHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) # Handle Okta's one-time verification challenge # Okta sends a GET request with x-okta-verification-challenge header # We need to return the challenge value in a JSON response + log.info("The OktaSetupHandler has been called with the #{env['REQUEST_METHOD']} method") + verification_challenge = extract_verification_challenge(headers) if verification_challenge diff --git a/spec/acceptance/plugins/handlers/slack_handler.rb b/spec/acceptance/plugins/handlers/slack_handler.rb index b2e053a..ae26170 100644 --- a/spec/acceptance/plugins/handlers/slack_handler.rb +++ b/spec/acceptance/plugins/handlers/slack_handler.rb @@ -1,7 +1,7 @@ # frozen_string_literal: true class SlackHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) return { status: "success" } diff --git a/spec/acceptance/plugins/handlers/team1_handler.rb b/spec/acceptance/plugins/handlers/team1_handler.rb index 9973446..6cf1493 100644 --- a/spec/acceptance/plugins/handlers/team1_handler.rb +++ b/spec/acceptance/plugins/handlers/team1_handler.rb @@ -8,7 +8,7 @@ class Team1Handler < Hooks::Plugins::Handlers::Base # @param headers [Hash] HTTP headers # @param config [Hash] Endpoint configuration # @return [Hash] Response data - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) log.debug("got a call to #{self.class.name} with payload: #{payload.inspect}") # demo the global retryable instance is a kinda silly way diff --git a/spec/acceptance/plugins/handlers/test_handler.rb b/spec/acceptance/plugins/handlers/test_handler.rb index 16117dc..f4d2ee1 100644 --- a/spec/acceptance/plugins/handlers/test_handler.rb +++ b/spec/acceptance/plugins/handlers/test_handler.rb @@ -1,11 +1,12 @@ # frozen_string_literal: true class TestHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { status: "test_success", handler: "TestHandler", payload_received: payload, + env_received: env, config_opts: config[:opts], timestamp: Time.now.utc.iso8601 } diff --git a/spec/integration/global_lifecycle_hooks_spec.rb b/spec/integration/global_lifecycle_hooks_spec.rb index 53e7e8b..da46b8c 100644 --- a/spec/integration/global_lifecycle_hooks_spec.rb +++ b/spec/integration/global_lifecycle_hooks_spec.rb @@ -81,7 +81,7 @@ def on_error(exception, env) # Create a test handler plugin that uses stats and failbot handler_plugin_content = <<~RUBY class IntegrationTestHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) stats.increment("handler.called", { handler: "IntegrationTestHandler" }) if payload&.dig("should_fail") diff --git a/spec/integration/hooks_integration_spec.rb b/spec/integration/hooks_integration_spec.rb index 14326fd..19080aa 100644 --- a/spec/integration/hooks_integration_spec.rb +++ b/spec/integration/hooks_integration_spec.rb @@ -43,7 +43,7 @@ def app require_relative "../../../../lib/hooks/plugins/handlers/base" class TestHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { status: "test_success", payload_received: payload, diff --git a/spec/unit/app/rack_env_builder_spec.rb b/spec/unit/app/rack_env_builder_spec.rb new file mode 100644 index 0000000..829214c --- /dev/null +++ b/spec/unit/app/rack_env_builder_spec.rb @@ -0,0 +1,301 @@ +# frozen_string_literal: true + +require "ostruct" +require_relative "../spec_helper" + +describe Hooks::App::RackEnvBuilder do + let(:request) { double("Grape::Request") } + let(:headers) do + { + "Content-Type" => "application/json", + "User-Agent" => "RSpec/Test", + "X-Custom-Header" => "custom-value", + "Authorization" => "Bearer token123" + } + end + let(:request_context) do + { + request_id: "req-123", + handler: "TestHandler" + } + end + let(:endpoint_config) do + { + path: "/test", + method: "post", + auth: true + } + end + let(:start_time) { Time.parse("2025-06-16T10:30:45Z") } + let(:full_path) { "/api/v1/test" } + + let(:builder) do + described_class.new( + request, + headers, + request_context, + endpoint_config, + start_time, + full_path + ) + end + + before do + # Mock the request object with all the methods used by the builder + allow(request).to receive(:request_method).and_return("POST") + allow(request).to receive(:path_info).and_return("/api/v1/test") + allow(request).to receive(:query_string).and_return("param1=value1¶m2=value2") + allow(request).to receive(:url).and_return("https://example.com/api/v1/test?param1=value1¶m2=value2") + allow(request).to receive(:content_type).and_return("application/json") + allow(request).to receive(:content_length).and_return("123") + allow(request).to receive(:env).and_return({ + "HTTP_VERSION" => "HTTP/1.1", + "SERVER_NAME" => "example.com", + "SERVER_PORT" => "443", + "REMOTE_ADDR" => "192.168.1.100" + }) + end + + describe "#initialize" do + it "stores all required parameters as instance variables" do + expect(builder.instance_variable_get(:@request)).to eq(request) + expect(builder.instance_variable_get(:@headers)).to eq(headers) + expect(builder.instance_variable_get(:@request_context)).to eq(request_context) + expect(builder.instance_variable_get(:@endpoint_config)).to eq(endpoint_config) + expect(builder.instance_variable_get(:@start_time)).to eq(start_time) + expect(builder.instance_variable_get(:@full_path)).to eq(full_path) + end + end + + describe "#build" do + let(:result) { builder.build } + + it "returns a hash with all standard Rack environment variables" do + expect(result).to be_a(Hash) + expect(result["REQUEST_METHOD"]).to eq("POST") + expect(result["PATH_INFO"]).to eq("/api/v1/test") + expect(result["QUERY_STRING"]).to eq("param1=value1¶m2=value2") + expect(result["HTTP_VERSION"]).to eq("HTTP/1.1") + expect(result["REQUEST_URI"]).to eq("https://example.com/api/v1/test?param1=value1¶m2=value2") + expect(result["SERVER_NAME"]).to eq("example.com") + expect(result["SERVER_PORT"]).to eq("443") + expect(result["CONTENT_TYPE"]).to eq("application/json") + expect(result["CONTENT_LENGTH"]).to eq("123") + expect(result["REMOTE_ADDR"]).to eq("192.168.1.100") + end + + it "includes Hooks-specific environment variables" do + expect(result["hooks.request_id"]).to eq("req-123") + expect(result["hooks.handler"]).to eq("TestHandler") + expect(result["hooks.endpoint_config"]).to eq(endpoint_config) + expect(result["hooks.start_time"]).to eq("2025-06-16T10:30:45Z") + expect(result["hooks.full_path"]).to eq("/api/v1/test") + end + + it "converts HTTP headers to Rack environment format" do + expect(result["HTTP_CONTENT_TYPE"]).to eq("application/json") + expect(result["HTTP_USER_AGENT"]).to eq("RSpec/Test") + expect(result["HTTP_X_CUSTOM_HEADER"]).to eq("custom-value") + expect(result["HTTP_AUTHORIZATION"]).to eq("Bearer token123") + end + + context "with hyphenated header names" do + let(:headers) do + { + "X-Forwarded-For" => "10.0.0.1", + "Accept-Language" => "en-US,en;q=0.9" + } + end + + it "converts hyphens to underscores in header names" do + expect(result["HTTP_X_FORWARDED_FOR"]).to eq("10.0.0.1") + expect(result["HTTP_ACCEPT_LANGUAGE"]).to eq("en-US,en;q=0.9") + end + end + + context "with lowercase header names" do + let(:headers) do + { + "content-type" => "text/plain", + "accept" => "text/html" + } + end + + it "converts header names to uppercase" do + expect(result["HTTP_CONTENT_TYPE"]).to eq("text/plain") + expect(result["HTTP_ACCEPT"]).to eq("text/html") + end + end + + context "with empty headers" do + let(:headers) { {} } + + it "still builds base environment without HTTP headers" do + expect(result["REQUEST_METHOD"]).to eq("POST") + expect(result["hooks.request_id"]).to eq("req-123") + # Note: HTTP_VERSION is from request.env, not from headers + http_headers = result.keys.grep(/^HTTP_/).reject { |k| k == "HTTP_VERSION" } + expect(http_headers).to be_empty + end + end + + context "with nil values in request" do + before do + allow(request).to receive(:content_type).and_return(nil) + allow(request).to receive(:content_length).and_return(nil) + allow(request).to receive(:query_string).and_return("") + allow(request).to receive(:env).and_return({ + "HTTP_VERSION" => "HTTP/1.1", + "SERVER_NAME" => "example.com", + "SERVER_PORT" => nil, + "REMOTE_ADDR" => "192.168.1.100" + }) + end + + it "handles nil values gracefully" do + expect(result["CONTENT_TYPE"]).to be_nil + expect(result["CONTENT_LENGTH"]).to be_nil + expect(result["SERVER_PORT"]).to be_nil + expect(result["QUERY_STRING"]).to eq("") + end + end + + context "with empty string values for numeric fields" do + before do + allow(request).to receive(:content_length).and_return("") + allow(request).to receive(:env).and_return({ + "HTTP_VERSION" => "HTTP/1.1", + "SERVER_NAME" => "example.com", + "SERVER_PORT" => "", + "REMOTE_ADDR" => "192.168.1.100" + }) + end + + it "handles empty string values gracefully by converting them to nil" do + expect(result["CONTENT_LENGTH"]).to eq("") + expect(result["SERVER_PORT"]).to eq("") + end + end + + context "with string numeric values" do + before do + allow(request).to receive(:content_length).and_return("456") + allow(request).to receive(:env).and_return({ + "HTTP_VERSION" => "HTTP/1.1", + "SERVER_NAME" => "example.com", + "SERVER_PORT" => "8080", + "REMOTE_ADDR" => "192.168.1.100" + }) + end + end + + context "with different request methods" do + before do + allow(request).to receive(:request_method).and_return("GET") + end + + it "captures the correct request method" do + expect(result["REQUEST_METHOD"]).to eq("GET") + end + end + + context "with complex endpoint configuration" do + let(:endpoint_config) do + { + path: "/webhooks/:id", + method: "put", + auth: { + type: "hmac", + secret_key: "webhook_secret" + }, + rate_limit: { + requests: 100, + window: 3600 + } + } + end + + it "includes the complete endpoint configuration" do + expect(result["hooks.endpoint_config"]).to eq(endpoint_config) + expect(result["hooks.endpoint_config"][:auth][:type]).to eq("hmac") + expect(result["hooks.endpoint_config"][:rate_limit][:requests]).to eq(100) + end + end + + context "with edge case header values" do + let(:headers) do + { + "Empty-Header" => "", + "Multi-Value" => "value1, value2, value3", + "Special-Chars" => "value with spaces & symbols!", + "Unicode-Header" => "héllo wörld 🌍" + } + end + + it "preserves header values exactly" do + expect(result["HTTP_EMPTY_HEADER"]).to eq("") + expect(result["HTTP_MULTI_VALUE"]).to eq("value1, value2, value3") + expect(result["HTTP_SPECIAL_CHARS"]).to eq("value with spaces & symbols!") + expect(result["HTTP_UNICODE_HEADER"]).to eq("héllo wörld 🌍") + end + end + end + + describe "private methods" do + describe "#build_base_environment" do + it "is called when building the environment" do + expect(builder).to receive(:build_base_environment).and_call_original + builder.build + end + end + + describe "#add_http_headers" do + it "is called when building the environment" do + expect(builder).to receive(:add_http_headers).and_call_original + builder.build + end + end + end + + describe "integration with real request-like objects" do + context "when used with objects that respond to expected methods" do + let(:mock_request) do + OpenStruct.new( + request_method: "PATCH", + path_info: "/api/v2/resource", + query_string: "filter=active", + url: "https://api.example.com/api/v2/resource?filter=active", + content_type: "application/vnd.api+json", + content_length: "456", + env: { + "HTTP_VERSION" => "HTTP/2.0", + "SERVER_NAME" => "api.example.com", + "SERVER_PORT" => "443", + "REMOTE_ADDR" => "203.0.113.5" + } + ) + end + + let(:builder_with_mock) do + described_class.new( + mock_request, + { "Accept" => "application/vnd.api+json" }, + { request_id: "mock-123", handler: "MockHandler" }, + { path: "/resource", method: "patch" }, + Time.parse("2025-06-16T15:45:30Z"), + "/api/v2/resource" + ) + end + + it "works correctly with mock objects" do + result = builder_with_mock.build + + expect(result["REQUEST_METHOD"]).to eq("PATCH") + expect(result["PATH_INFO"]).to eq("/api/v2/resource") + expect(result["HTTP_VERSION"]).to eq("HTTP/2.0") + expect(result["HTTP_ACCEPT"]).to eq("application/vnd.api+json") + expect(result["hooks.request_id"]).to eq("mock-123") + end + end + end +end diff --git a/spec/unit/lib/hooks/app/auth/auth_security_spec.rb b/spec/unit/lib/hooks/app/auth/auth_security_spec.rb index e56435d..b94d8b7 100644 --- a/spec/unit/lib/hooks/app/auth/auth_security_spec.rb +++ b/spec/unit/lib/hooks/app/auth/auth_security_spec.rb @@ -27,9 +27,14 @@ def error!(message, code) context "with missing auth configuration" do it "rejects request with no auth config" do endpoint_config = {} + global_config = {} + request_context = { request_id: "test-request-id" } + log_msg = "authentication configuration missing or invalid - request_id: #{request_context[:request_id]}" + + expect(log).to receive(:error).with(log_msg) expect do - instance.validate_auth!(payload, headers, endpoint_config) + instance.validate_auth!(payload, headers, endpoint_config, global_config, request_context) end.to raise_error(StandardError, /authentication configuration missing or invalid/) end diff --git a/spec/unit/lib/hooks/app/helpers_security_spec.rb b/spec/unit/lib/hooks/app/helpers_security_spec.rb index 0d4accc..d01a05d 100644 --- a/spec/unit/lib/hooks/app/helpers_security_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_security_spec.rb @@ -38,7 +38,7 @@ def env it "returns error indicating handler not found" do expect do instance.load_handler("NonexistentHandler") - end.to raise_error(StandardError, /failed to get handler.*not found/) + end.to raise_error(StandardError, /Handler plugin.*not found/) end it "returns error for any handler class name when no plugins loaded" do @@ -47,7 +47,7 @@ def env handler_names.each do |name| expect do instance.load_handler(name) - end.to raise_error(StandardError, /failed to get handler.*not found/) + end.to raise_error(StandardError, /Handler plugin.*not found/) end end end diff --git a/spec/unit/lib/hooks/app/helpers_spec.rb b/spec/unit/lib/hooks/app/helpers_spec.rb index 8925c51..7e7bb66 100644 --- a/spec/unit/lib/hooks/app/helpers_spec.rb +++ b/spec/unit/lib/hooks/app/helpers_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "tempfile" +require "json" require_relative "../../../spec_helper" describe Hooks::App::Helpers do @@ -23,7 +24,7 @@ def request end def error!(message, code) - raise StandardError, "#{code}: #{message}" + raise StandardError, "#{code}: #{message.to_json}" end end end @@ -57,8 +58,20 @@ def error!(message, code) it "raises error when content length exceeds limit" do helper.headers["Content-Length"] = "1500" - - expect { helper.enforce_request_limits(config) }.to raise_error(StandardError, /413.*too large/) + request_context = { request_id: "test-request-id" } + + error = nil + begin + helper.enforce_request_limits(config, request_context) + rescue StandardError => e + error = e + end + + expect(error).to be_a(StandardError) + expect(error.message).to start_with("413: ") + body = error.message.sub("413: ", "") + parsed = JSON.parse(body) + expect(parsed).to eq({ "error" => "request_body_too_large", "message" => "request body too large", "request_id" => "test-request-id" }) end end @@ -283,7 +296,7 @@ def error!(message, code) it "returns error indicating handler not found" do expect do helper.load_handler("NonexistentHandler") - end.to raise_error(StandardError, /failed to get handler.*not found/) + end.to raise_error(StandardError, /Handler plugin.*not found/) end end diff --git a/spec/unit/lib/hooks/core/plugin_loader_spec.rb b/spec/unit/lib/hooks/core/plugin_loader_spec.rb index 3def008..b85bcec 100644 --- a/spec/unit/lib/hooks/core/plugin_loader_spec.rb +++ b/spec/unit/lib/hooks/core/plugin_loader_spec.rb @@ -68,7 +68,7 @@ def self.valid?(payload:, headers:, config:) let(:custom_handler_content) do <<~RUBY class CustomHandler < Hooks::Plugins::Handlers::Base - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { message: "custom handler executed", payload: payload } end end @@ -114,7 +114,7 @@ def call(payload:, headers:, config:) custom_handler_class = described_class.handler_plugins["CustomHandler"] expect(custom_handler_class).to be < Hooks::Plugins::Handlers::Base handler_instance = custom_handler_class.new - result = handler_instance.call(payload: "test", headers: {}, config: {}) + result = handler_instance.call(payload: "test", headers: {}, env: {}, config: {}) expect(result).to include(message: "custom handler executed", payload: "test") end end @@ -288,7 +288,7 @@ def self.valid?(payload:, headers:, config:) wrong_file = File.join(temp_handler_dir, "wrong_handler.rb") File.write(wrong_file, <<~RUBY) class WrongHandler - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { message: "wrong handler" } end end diff --git a/spec/unit/lib/hooks/handlers/base_spec.rb b/spec/unit/lib/hooks/handlers/base_spec.rb index 1ce807c..c1ad7b6 100644 --- a/spec/unit/lib/hooks/handlers/base_spec.rb +++ b/spec/unit/lib/hooks/handlers/base_spec.rb @@ -6,16 +6,22 @@ let(:payload) { { "data" => "test" } } let(:headers) { { "Content-Type" => "application/json" } } let(:config) { { "endpoint" => "/test" } } + let(:env) do + { + "REQUEST_METHOD" => "GET", + "hooks.request_id" => "fake-request-id", + } + end it "raises NotImplementedError by default" do expect { - handler.call(payload: payload, headers: headers, config: config) + handler.call(payload: payload, headers: headers, env: env, config: config) }.to raise_error(NotImplementedError, "Handler must implement #call method") end it "can be subclassed and overridden" do test_handler_class = Class.new(described_class) do - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { received_payload: payload, received_headers: headers, @@ -26,7 +32,7 @@ def call(payload:, headers:, config:) end handler = test_handler_class.new - result = handler.call(payload: payload, headers: headers, config: config) + result = handler.call(payload: payload, headers: headers, env: env, config: config) expect(result).to eq({ received_payload: payload, @@ -38,7 +44,7 @@ def call(payload:, headers:, config:) it "accepts different payload types" do test_handler_class = Class.new(described_class) do - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { payload_class: payload.class.name } end end @@ -46,21 +52,21 @@ def call(payload:, headers:, config:) handler = test_handler_class.new # Test with hash - result = handler.call(payload: { "test" => "data" }, headers: headers, config: config) + result = handler.call(payload: { "test" => "data" }, headers: headers, env: env, config: config) expect(result[:payload_class]).to eq("Hash") # Test with string - result = handler.call(payload: "raw string", headers: headers, config: config) + result = handler.call(payload: "raw string", headers: headers, env: env, config: config) expect(result[:payload_class]).to eq("String") # Test with nil - result = handler.call(payload: nil, headers: headers, config: config) + result = handler.call(payload: nil, headers: headers, env: env, config: config) expect(result[:payload_class]).to eq("NilClass") end it "accepts different header types" do test_handler_class = Class.new(described_class) do - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { headers_received: headers } end end @@ -69,21 +75,21 @@ def call(payload:, headers:, config:) # Test with hash headers_hash = { "User-Agent" => "test", "X-Custom" => "value" } - result = handler.call(payload: payload, headers: headers_hash, config: config) + result = handler.call(payload: payload, headers: headers_hash, env: env, config: config) expect(result[:headers_received]).to eq(headers_hash) # Test with empty hash - result = handler.call(payload: payload, headers: {}, config: config) + result = handler.call(payload: payload, headers: {}, env: env, config: config) expect(result[:headers_received]).to eq({}) # Test with nil - result = handler.call(payload: payload, headers: nil, config: config) + result = handler.call(payload: payload, headers: nil, env: env, config: config) expect(result[:headers_received]).to be_nil end it "accepts different config types" do test_handler_class = Class.new(described_class) do - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) { config_received: config } end end @@ -96,25 +102,25 @@ def call(payload:, headers:, config:) "opts" => { "timeout" => 30 }, "handler" => "TestHandler" } - result = handler.call(payload: payload, headers: headers, config: complex_config) + result = handler.call(payload: payload, headers: headers, env: env, config: complex_config) expect(result[:config_received]).to eq(complex_config) # Test with empty config - result = handler.call(payload: payload, headers: headers, config: {}) + result = handler.call(payload: payload, headers: headers, env: env, config: {}) expect(result[:config_received]).to eq({}) end it "requires all keyword arguments" do expect { - handler.call(payload: payload, headers: headers) + handler.call(payload: payload, headers: headers, env: env) }.to raise_error(ArgumentError, /missing keyword.*config/) expect { - handler.call(payload: payload, config: config) + handler.call(payload: payload, env: env, config: config) }.to raise_error(ArgumentError, /missing keyword.*headers/) expect { - handler.call(headers: headers, config: config) + handler.call(headers: headers, env: env, config: config) }.to raise_error(ArgumentError, /missing keyword.*payload/) end end @@ -127,7 +133,7 @@ def call(payload:, headers:, config:) it "maintains method signature in subclasses" do child_class = Class.new(described_class) do - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) "child implementation" end end @@ -136,6 +142,7 @@ def call(payload:, headers:, config:) result = handler.call( payload: { "test" => "data" }, headers: { "Content-Type" => "application/json" }, + env: { "REQUEST_METHOD" => "POST", "hooks.request_id" => "test-id" }, config: { "endpoint" => "/test" } ) @@ -181,7 +188,7 @@ def call(payload:, headers:, config:) it "allows stats and failbot usage in subclasses" do test_handler_class = Class.new(described_class) do - def call(payload:, headers:, config:) + def call(payload:, headers:, env:, config:) stats.increment("handler.called", { handler: "TestHandler" }) if payload.nil? @@ -225,14 +232,14 @@ def report(error_or_message, context = {}) handler = test_handler_class.new # Test with non-nil payload - handler.call(payload: { "test" => "data" }, headers: {}, config: {}) + handler.call(payload: { "test" => "data" }, headers: {}, env: {}, config: {}) expect(collected_data).to include( { type: :stats, action: :increment, metric: "handler.called", tags: { handler: "TestHandler" } } ) # Test with nil payload collected_data.clear - handler.call(payload: nil, headers: {}, config: {}) + handler.call(payload: nil, headers: {}, env: {}, config: {}) expect(collected_data).to match_array([ { type: :stats, action: :increment, metric: "handler.called", tags: { handler: "TestHandler" } }, { type: :failbot, action: :report, message: "Payload is nil", context: { handler: "TestHandler" } } diff --git a/spec/unit/lib/hooks/handlers/default_spec.rb b/spec/unit/lib/hooks/handlers/default_spec.rb index 5a96a43..5dd0b1f 100644 --- a/spec/unit/lib/hooks/handlers/default_spec.rb +++ b/spec/unit/lib/hooks/handlers/default_spec.rb @@ -6,6 +6,12 @@ let(:headers) { { "X-GitHub-Event" => "pull_request" } } let(:config) { { environment: "test" } } let(:handler) { described_class.new } + let(:env) do + { + "REQUEST_METHOD" => "GET", + "hooks.request_id" => "fake-request-id", + } + end before do allow(handler).to receive(:log).and_return(log) @@ -13,7 +19,7 @@ describe "#call" do context "with valid parameters" do - let(:result) { handler.call(payload:, headers:, config:) } + let(:result) { handler.call(payload:, headers:, env:, config:) } it "logs that the default handler was invoked" do result @@ -41,9 +47,10 @@ end end - context "with nil payload" do + context "with nil payload and nil env" do let(:payload) { nil } - let(:result) { handler.call(payload:, headers:, config:) } + let(:env) { nil } + let(:result) { handler.call(payload:, headers:, env:, config:) } it "does not log payload debug information" do result @@ -61,7 +68,7 @@ context "with empty payload" do let(:payload) { {} } - let(:result) { handler.call(payload:, headers:, config:) } + let(:result) { handler.call(payload:, headers:, env:, config:) } it "logs the empty payload" do result @@ -81,7 +88,7 @@ } } end - let(:result) { handler.call(payload:, headers:, config:) } + let(:result) { handler.call(payload:, headers:, env:, config:) } it "logs the complex payload structure" do result diff --git a/vendor/cache/ostruct-0.6.1.gem b/vendor/cache/ostruct-0.6.1.gem new file mode 100644 index 0000000..95788af Binary files /dev/null and b/vendor/cache/ostruct-0.6.1.gem differ