Merge pull request #62 from github/copilot/fix-61

task: default to JSON in and JSON out

Author: GrantBirki

Gemfile.lock

@@ -1,7 +1,7 @@
 PATH
   remote: .
   specs:
-    hooks-ruby (0.3.2)
+    hooks-ruby (0.4.0)
       dry-schema (~> 1.14, >= 1.14.1)
       grape (~> 2.3)
       puma (~> 6.6)

docs/configuration.md

@@ -103,7 +103,15 @@ When set to `true`, enables a catch-all route that will handle requests to unkno
 
 When set to `true`, normalizes incoming HTTP headers by lowercasing and trimming them. This ensures consistency in header names and values.
 
-**Default:** `true`  
+**Default:** `true`
+
+### `default_format`
+
+Sets the default response format when no specific format is requested.
+
+**Default:** `json`  
+**Valid values:** `json`, `txt`, `xml`, `any`  
+**Example:** `json`
 
 ## Endpoint Options
 

docs/handler_plugins.md

@@ -4,26 +4,33 @@ This document provides in-depth information about handler plugins and how you ca
 
 ## Writing a Handler Plugin
 
-Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base` class. They are used to process webhook payloads and can do anything you want. They follow a very simple interface that allows you to define a `call` method that takes three parameters: `payload`, `headers`, and `config`. The `call` method should return a hash with the response data. The hash that this method returns will be sent back to the webhook sender as a JSON response.
+Handler plugins are Ruby classes that extend the `Hooks::Plugins::Handlers::Base` class. They are used to process webhook payloads and can do anything you want. They follow a very simple interface that allows you to define a `call` method that takes four parameters: `payload`, `headers`, `env`, and `config`.
+
+**Important:** The `call` method should return a hash by default. Since the server now defaults to JSON format, any hash returned by the handler will be automatically converted to JSON with the correct `Content-Type: application/json` headers set by Grape. This ensures consistent API responses and proper JSON serialization.
 
 - `payload`: The webhook payload, which can be a Hash or a String. This is the data that the webhook sender sends to your endpoint.
 - `headers`: A Hash of HTTP headers that were sent with the webhook request.
 - `env`: A modified Rack environment that contains a lot of context about the request. This includes information about the request method, path, query parameters, and more. See [`rack_env_builder.rb`](../lib/hooks/app/rack_env_builder.rb) for the complete list of available keys.
 - `config`: A Hash containing 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.
 
+The method should return a **hash** that will be automatically serialized to JSON format with appropriate headers. The server defaults to JSON format for both input and output processing.
+
 ```ruby
 # example file path: plugins/handlers/example.rb
 class Example < Hooks::Plugins::Handlers::Base
   # Process a webhook payload
   #
   # @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 env [Hash] A modified Rack environment that contains a lot of context about the request
   # @param config [Hash] Endpoint configuration
-  # @return [Hash] Response data
+  # @return [Hash] Response data (automatically converted to JSON)
   def call(payload:, headers:, env:, config:)
+    # Return a hash - it will be automatically converted to JSON with proper headers
     return {
-      status: "success"
+      status: "success",
+      message: "webhook processed successfully",
+      timestamp: Time.now.iso8601
     }
   end
 end
@@ -43,6 +50,31 @@ It should be noted that the `handler:` key in the endpoint configuration file sh
 - `MyCustomHandler` -> `my_custom_handler`
 - `Cool2Handler` -> `cool_2_handler`
 
+## Default JSON Format
+
+By default, the Hooks server uses JSON format for both input and output processing. This means:
+
+- **Input**: Webhook payloads are parsed as JSON and passed to handlers as Ruby hashes
+- **Output**: Handler return values (hashes) are automatically converted to JSON responses with `Content-Type: application/json` headers
+- **Error Responses**: Authentication failures and handler errors return structured JSON responses
+
+**Best Practice**: Always return a hash from your handler's `call` method. The hash will be automatically serialized to JSON and sent to the webhook sender with proper headers. This ensures consistent API responses and proper JSON formatting.
+
+Example response format:
+
+```json
+{
+  "status": "success",
+  "message": "webhook processed successfully", 
+  "data": {
+    "processed_at": "2023-10-01T12:34:56Z",
+    "items_processed": 5
+  }
+}
+```
+
+> **Note**: The JSON format behavior can be configured using the `default_format` option in your global configuration. See the [Configuration documentation](./configuration.md) for more details.
+
 ### `payload` Parameter
 
 The `payload` parameter can be a Hash or a String. If the payload is a String, it will be parsed as JSON. If it is a Hash, it will be passed directly to the handler. The payload can contain any data that the webhook sender wants to send.
@@ -159,6 +191,8 @@ The `log.debug`, `log.info`, `log.warn`, and `log.error` methods are available i
 
 All handler plugins have access to the `error!` method, which is used to raise an error with a specific message and HTTP status code. This is useful for returning error responses to the webhook sender.
 
+When using `error!` with the default JSON format, both hash and string responses are handled appropriately:
+
 ```ruby
 class Example < Hooks::Plugins::Handlers::Base
   # Example webhook handler
@@ -167,11 +201,12 @@ class Example < Hooks::Plugins::Handlers::Base
   # @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
+  # @return [Hash] Response data (automatically converted to JSON)
   def call(payload:, headers:, env:, config:)
 
     if payload.nil? || payload.empty?
       log.error("Payload is empty or nil")
+      # String errors are JSON-encoded with default format
       error!("Payload cannot be empty or nil", 400)
     end
 
@@ -182,21 +217,22 @@ class Example < Hooks::Plugins::Handlers::Base
 end
 ```
 
-You can also use the `error!` method to return a JSON response as well:
+**Recommended approach**: Use hash-based error responses for consistent JSON structure:
 
 ```ruby
 class Example < Hooks::Plugins::Handlers::Base
   def call(payload:, headers:, env:, config:)
 
     if payload.nil? || payload.empty?
       log.error("Payload is empty or nil")
+      # Hash errors are automatically converted to JSON
       error!({
         error: "payload_empty",
         message: "the payload cannot be empty or nil",
         success: false,
         custom_value: "some_custom_value",
         request_id: env["hooks.request_id"]
-      }, 500)
+      }, 400)
     end
 
     return {
@@ -206,6 +242,18 @@ class Example < Hooks::Plugins::Handlers::Base
 end
 ```
 
+This will return a properly formatted JSON error response:
+
+```json
+{
+  "error": "payload_empty",
+  "message": "the payload cannot be empty or nil",
+  "success": false,
+  "custom_value": "some_custom_value",
+  "request_id": "uuid-here"
+}
+```
+
 ### `#Retryable.with_context(:default)`
 
 This method uses a default `Retryable` context to handle retries. It is used to wrap the execution of a block of code that may need to be retried in case of failure.
@@ -220,7 +268,7 @@ class Example < Hooks::Plugins::Handlers::Base
   # @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
+  # @return [Hash] Response data (automatically converted to JSON)
   def call(payload:, headers:, env:, config:)
     result = Retryable.with_context(:default) do
       some_operation_that_might_fail()
@@ -229,7 +277,9 @@ class Example < Hooks::Plugins::Handlers::Base
     log.debug("operation result: #{result}")
 
     return {
-      status: "success"
+      status: "success",
+      operation_result: result,
+      processed_at: Time.now.iso8601
     }
   end
 end

lib/hooks/app/api.rb

@@ -39,8 +39,7 @@ def self.create(config:, endpoints:, log:)
           content_type :xml, "application/xml"
           content_type :any, "*/*"
 
-          format :txt # TODO: make this configurable via config[:format] (defaults to :json in the future)
-          default_format :txt # TODO: make this configurable via config[:default_format] (defaults to :json in the future)
+          default_format config[:default_format] || :json
         end
 
         api_class.class_eval do
@@ -118,22 +117,21 @@ def self.create(config:, endpoints:, log:)
                   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.to_json
+                  response
                 rescue Hooks::Plugins::Handlers::Error => e
                   # Handler called error! method - immediately return error response and exit the request
                   log.debug("handler #{handler_class_name} called `error!` method")
 
-                  error_response = nil
-
                   status e.status
                   case e.body
                   when String
+                    # if error! was called with a string, we assume it's a simple text error
+                    # example: error!("simple text error", 400) -> should return a plain text response
                     content_type "text/plain"
                     error_response = e.body
                   else
-                    content_type "application/json"
-                    error_response = e.body.to_json
+                    # Let Grape handle JSON conversion with the default format
+                    error_response = e.body
                   end
 
                   return error_response
@@ -164,8 +162,7 @@ def self.create(config:, endpoints:, log:)
                   error_response[:handler] = handler_class_name unless config[:production]
 
                   status determine_error_code(e)
-                  content_type "application/json"
-                  error_response.to_json
+                  error_response
                 end
               end
             end

lib/hooks/app/endpoints/catchall.rb

@@ -17,6 +17,13 @@ module App
     class CatchallEndpoint < Grape::API
       include Hooks::App::Helpers
 
+      # Set up content types and default format to JSON to match main API
+      content_type :json, "application/json"
+      content_type :txt, "text/plain"
+      content_type :xml, "application/xml"
+      content_type :any, "*/*"
+      default_format :json
+
       def self.mount_path(config)
         # :nocov:
         "#{config[:root_path]}/*path"
@@ -81,8 +88,7 @@ def self.route_block(captured_config, captured_logger)
               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.to_json
+              response
             rescue StandardError => e
               err_msg = "Error processing webhook event with handler: #{handler_class_name} - #{e.message} " \
                 "- request_id: #{request_id} - path: #{full_path} - method: #{http_method} - " \
@@ -102,8 +108,7 @@ def self.route_block(captured_config, captured_logger)
               error_response[:handler] = handler_class_name unless config[:production]
 
               status determine_error_code(e)
-              content_type "application/json"
-              error_response.to_json
+              error_response
             end
           end
         end

lib/hooks/app/endpoints/health.rb

@@ -6,14 +6,20 @@
 module Hooks
   module App
     class HealthEndpoint < Grape::API
+      # Set up content types and default format to JSON
+      content_type :json, "application/json"
+      content_type :txt, "text/plain"
+      content_type :xml, "application/xml"
+      content_type :any, "*/*"
+      default_format :json
+
       get do
-        content_type "application/json"
         {
           status: "healthy",
           timestamp: Time.now.utc.iso8601,
           version: Hooks::VERSION,
           uptime_seconds: (Time.now - Hooks::App::API.server_start_time).to_i
-        }.to_json
+        }
       end
     end
   end

lib/hooks/app/endpoints/version.rb

@@ -6,12 +6,18 @@
 module Hooks
   module App
     class VersionEndpoint < Grape::API
+      # Set up content types and default format to JSON
+      content_type :json, "application/json"
+      content_type :txt, "text/plain"
+      content_type :xml, "application/xml"
+      content_type :any, "*/*"
+      default_format :json
+
       get do
-        content_type "application/json"
         {
           version: Hooks::VERSION,
           timestamp: Time.now.utc.iso8601
-        }.to_json
+        }
       end
     end
   end

lib/hooks/core/config_loader.rb

@@ -20,7 +20,8 @@ class ConfigLoader
         production: true,
         endpoints_dir: "./config/endpoints",
         use_catchall_route: false,
-        normalize_headers: true
+        normalize_headers: true,
+        default_format: :json
       }.freeze
 
       SILENCE_CONFIG_LOADER_MESSAGES = ENV.fetch(
@@ -141,6 +142,7 @@ def self.load_env_config
           "HOOKS_ENDPOINTS_DIR" => :endpoints_dir,
           "HOOKS_USE_CATCHALL_ROUTE" => :use_catchall_route,
           "HOOKS_NORMALIZE_HEADERS" => :normalize_headers,
+          "HOOKS_DEFAULT_FORMAT" => :default_format,
           "HOOKS_SOME_STRING_VAR" => :some_string_var # Added for test
         }
 
@@ -155,6 +157,9 @@ def self.load_env_config
           when :use_catchall_route, :normalize_headers
             # Convert string to boolean
             env_config[config_key] = %w[true 1 yes on].include?(value.downcase)
+          when :default_format
+            # Convert string to symbol
+            env_config[config_key] = value.to_sym
           else
             env_config[config_key] = value
           end

lib/hooks/core/config_validator.rb

@@ -27,6 +27,7 @@ class ValidationError < StandardError; end
         optional(:endpoints_dir).filled(:string)
         optional(:use_catchall_route).filled(:bool)
         optional(:normalize_headers).filled(:bool)
+        optional(:default_format).filled(:symbol, included_in?: %i[json txt xml any])
 
         optional(:ip_filtering).hash do
           optional(:ip_header).filled(:string)

lib/hooks/version.rb

@@ -4,5 +4,5 @@
 module Hooks
   # Current version of the Hooks webhook framework
   # @return [String] The version string following semantic versioning
-  VERSION = "0.3.2".freeze
+  VERSION = "0.4.0".freeze
 end