Merge pull request #70 from github/config-prod-fixes

Config prod fixes and better logging

Author: GrantBirki

Gemfile.lock

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

lib/hooks/app/api.rb

@@ -31,6 +31,7 @@ class << self
       # Create a new configured API class
       def self.create(config:, endpoints:, log:)
         # :nocov:
+        @production = config[:environment].downcase.strip == "production"
         @server_start_time = Time.now
 
         api_class = Class.new(Grape::API) do
@@ -157,9 +158,9 @@ def self.create(config:, endpoints:, log:)
                   }
 
                   # 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]
+                  error_response[:backtrace] = e.backtrace.join("\n") unless @production
+                  error_response[:message] = e.message unless @production
+                  error_response[:handler] = handler_class_name unless @production
 
                   status determine_error_code(e)
                   error_response

lib/hooks/app/endpoints/catchall.rb

@@ -103,9 +103,9 @@ def self.route_block(captured_config, captured_logger)
               }
 
               # 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]
+              error_response[:backtrace] = e.backtrace.join("\n") unless @production
+              error_response[:message] = e.message unless @production
+              error_response[:handler] = handler_class_name unless @production
 
               status determine_error_code(e)
               error_response

lib/hooks/core/builder.rb

@@ -37,6 +37,8 @@ def build
           )
         end
 
+        @log.debug("global config loaded: #{config.inspect}")
+
         Hooks::Log.instance = @log
 
         # Register user-defined components globally
@@ -55,7 +57,6 @@ def build
         @log.info "starting hooks server v#{Hooks::VERSION}"
         @log.info "config: #{endpoints.size} endpoints loaded"
         @log.info "environment: #{config[:environment]}"
-        @log.info "production mode: #{config[:production]}"
         @log.info "available endpoints: #{endpoints.map { |e| e[:path] }.join(', ')}"
 
         # Build and return Grape API class

lib/hooks/core/config_loader.rb

@@ -69,12 +69,6 @@ def self.load(config_path: nil)
         # Convert string keys to symbols for consistency
         config = symbolize_keys(config)
 
-        if config[:environment].downcase == "production"
-          config[:production] = true
-        else
-          config[:production] = false
-        end
-
         # Log overrides if any were made
         if overrides.any?
           puts "INFO: Configuration overrides applied from: #{overrides.join(', ')}" unless SILENCE_CONFIG_LOADER_MESSAGES

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.6.2".freeze
+  VERSION = "0.6.3".freeze
 end

spec/unit/lib/hooks/core/builder_spec.rb

@@ -141,7 +141,7 @@
     end
 
     context "with custom logger" do
-      let(:custom_logger) { double("Logger", info: nil) }
+      let(:custom_logger) { double("Logger", info: nil, debug: nil) }
       let(:builder) { described_class.new(log: custom_logger) }
 
       before do
@@ -216,6 +216,7 @@
       let(:mock_logger) { double("Logger", info: nil) }
 
       before do
+        allow(mock_logger).to receive(:debug)
         allow(Hooks::Core::ConfigLoader).to receive(:load).and_return({
           log_level: "debug",
           environment: "test"
@@ -244,6 +245,7 @@
         expect(mock_logger).to receive(:info).with("config: 0 endpoints loaded")
         expect(mock_logger).to receive(:info).with("environment: test")
         expect(mock_logger).to receive(:info).with("available endpoints: ")
+        expect(mock_logger).to receive(:debug).with(/global config loaded: /)
 
         builder.build
       end
@@ -258,6 +260,7 @@
 
         expect(mock_logger).to receive(:info).with("config: 2 endpoints loaded")
         expect(mock_logger).to receive(:info).with("available endpoints: /webhook/test1, /webhook/test2")
+        expect(mock_logger).to receive(:debug).with(/global config loaded: /)
 
         builder.build
       end

spec/unit/lib/hooks/core/config_loader_spec.rb

@@ -33,7 +33,6 @@
 
         expect(config[:log_level]).to eq("debug")
         expect(config[:environment]).to eq("test")
-        expect(config[:production]).to be false # should be false when environment is test
         expect(config[:handler_plugin_dir]).to eq("./plugins/handlers") # defaults should remain
       end
     end
@@ -69,7 +68,6 @@
           expect(config[:log_level]).to eq("debug")
           expect(config[:environment]).to eq("development")
           expect(config[:request_timeout]).to eq(60)
-          expect(config[:production]).to be false
           expect(config[:handler_plugin_dir]).to eq("./plugins/handlers") # defaults should remain
         end
       end
@@ -94,7 +92,6 @@
           expect(config[:log_level]).to eq("warn")
           expect(config[:environment]).to eq("staging")
           expect(config[:endpoints_dir]).to eq("./custom/endpoints")
-          expect(config[:production]).to be false
         end
       end
 
@@ -157,7 +154,6 @@
         expect(config[:environment]).to eq("development")
         expect(config[:request_limit]).to eq(2_097_152)
         expect(config[:request_timeout]).to eq(45)
-        expect(config[:production]).to be false
       end
 
       it "handles partial environment variable overrides" do
@@ -167,7 +163,6 @@
 
         expect(config[:log_level]).to eq("warn")
         expect(config[:environment]).to eq("production") # should remain default
-        expect(config[:production]).to be true
         # Ensure other ENV vars are not set from previous examples in this context
         expect(ENV["HOOKS_ENVIRONMENT"]).to be_nil
         expect(ENV["HOOKS_REQUEST_LIMIT"]).to be_nil
@@ -240,14 +235,14 @@
       it "sets production to true when environment is production" do
         config = described_class.load(config_path: { environment: "production" })
 
-        expect(config[:production]).to be true
+        expect(config[:environment]).to eq("production")
       end
 
       it "sets production to false when environment is not production" do
         ["development", "test", "staging", "custom"].each do |env|
           config = described_class.load(config_path: { environment: env })
 
-          expect(config[:production]).to be false
+          expect(config[:environment]).to eq(env)
         end
       end
     end