Skip to content

Implement Grape's error! method for handler subclasses #50

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 11 commits into from
Jun 17, 2025

Conversation

Copilot
Copy link
Contributor

@Copilot Copilot AI commented Jun 16, 2025

This PR implements support for Grape's error! method in handler subclasses, allowing them to immediately terminate request processing and return custom error responses instead of raising generic StandardError exceptions.

Problem

Previously, handler subclasses could only raise standard exceptions which would be caught and converted to generic 500 errors with "server_error" messages. This made it impossible for handlers to return specific error codes and custom error responses. For example:

class BoomtownWithError < Hooks::Plugins::Handlers::Base
  def call(payload:, headers:, env:, config:)
    if payload["boom"] == true
      # This would fail with "undefined method 'error!'"
      error!({
        error: "boomtown_with_error",
        message: "the payload triggered a boomtown error",
        request_id: env["hooks.request_id"]
      }, 500)
    end
    { status: "ok" }
  end
end

Solution

The implementation uses a custom exception class to bridge the gap between handler classes and the Grape API context:

  1. Custom Exception Class: Created Hooks::Plugins::Handlers::Error to carry error details from handlers back to the API
  2. Handler Method: Added error!(body, status = 500) method to Hooks::Plugins::Handlers::Base with the same interface as Grape's native error!
  3. API Integration: Modified the API rescue block to catch handler errors before general StandardError handling

Usage

Handlers can now call error! with the same interface as Grape:

# Return a custom error with status 400
error!({ error: "validation_failed", message: "Invalid payload" }, 400)

# Return a simple string error with status 401  
error!("Unauthorized", 401)

# Return an error with default 500 status
error!({ error: "internal_error", message: "Something went wrong" })

Changes Made

  • New files:

    • lib/hooks/plugins/handlers/error.rb - Custom exception class
    • spec/unit/lib/hooks/handlers/error_spec.rb - Unit tests for exception class
  • Modified files:

    • lib/hooks/plugins/handlers/base.rb - Added error! method
    • lib/hooks/app/api.rb - Added rescue block for handler errors
    • spec/unit/lib/hooks/handlers/base_spec.rb - Unit tests for error! method
    • spec/acceptance/acceptance_tests.rb - Uncommented and fixed failing acceptance test
    • spec/acceptance/plugins/handlers/boomtown_with_error.rb - Updated TODO comment

Testing

  • Maintains 100% test coverage
  • All unit, integration, and acceptance tests pass
  • The previously failing acceptance test now passes successfully
  • Added comprehensive unit tests for new functionality

Fixes #49.


💬 Share your feedback on Copilot coding agent for the chance to win a $200 gift card! Click here to start the survey.

@Copilot Copilot AI changed the title [WIP] task: allow handler subclasses to call Grape's error! method Implement Grape's error! method for handler subclasses Jun 16, 2025
@Copilot Copilot AI requested a review from GrantBirki June 16, 2025 22:56
Copilot finished work on behalf of GrantBirki June 16, 2025 22:56
@GrantBirki GrantBirki marked this pull request as ready for review June 16, 2025 23:12
@Copilot Copilot AI review requested due to automatic review settings June 16, 2025 23:12
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull Request Overview

This PR adds Grape‐style error! support to handler subclasses by introducing a custom exception, wiring it into the handler base class, and catching it in the API layer to return structured error responses. It also includes corresponding unit and acceptance test updates.

  • Introduced Hooks::Plugins::Handlers::Error exception with body and status payload
  • Added error! method to Handlers::Base that raises the new exception
  • Updated Hooks::App::API to rescue Handlers::Error and return JSON responses
  • Expanded unit tests for the exception and error! method; fixed acceptance tests for the new behavior

Reviewed Changes

Copilot reviewed 10 out of 8 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
spec/unit/lib/hooks/handlers/error_spec.rb Added tests verifying Error initialization behavior
spec/unit/lib/hooks/handlers/base_spec.rb Added tests for Base#error! raising Error
spec/acceptance/plugins/handlers/boomtown_with_error.rb Updated comment to reflect new error! usage
spec/acceptance/acceptance_tests.rb Rewrote acceptance test to expect custom handler error
lib/hooks/plugins/handlers/error.rb Implemented custom Error exception
lib/hooks/plugins/handlers/base.rb Added error! method to handler base class
lib/hooks/app/api.rb Caught Handlers::Error in API and returned JSON errors
.bundle/config CI environment paths mistakenly committed
Comments suppressed due to low confidence (1)

.bundle/config:3

  • [nitpick] This CI/workflow–specific change to .bundle/config was committed; environment-specific paths should not be in version control. Consider reverting these lines or adding .bundle/config to .gitignore.
BUNDLE_PATH: "/home/runner/work/hooks/hooks/vendor/bundle"

@GrantBirki
Copy link
Member

@copilot everything looks really good and works great on my machine running Ruby 3.4.4 (the latest version of Ruby). However, I see that several older versions are failing when running script/test with the following error:

🧪 Running tests: 23:30:29

..............................................................................................................................................................................................................................................................................................................................................F................................................................................................................................................................................................................

Failures:

  1) Hooks::Plugins::Handlers::Error#initialize creates an error with custom status
     Failure/Error: expect(error.message).to eq("Handler error: 400 - {error: \"validation_failed\"}")

       expected: "Handler error: 400 - {error: \"validation_failed\"}"
            got: "Handler error: 400 - {:error=>\"validation_failed\"}"

       (compared using ==)
     # ./spec/unit/lib/hooks/handlers/error_spec.rb:1[6](https://github.com/github/hooks/actions/runs/15694239021/job/44215973731?pr=50#step:5:7):in `block (3 levels) in <top (required)>'

Finished in 0.[7](https://github.com/github/hooks/actions/runs/15694239021/job/44215973731?pr=50#step:5:8)6709 seconds (files took 0.72901 seconds to load)
543 examples, 1 failure

Failed examples:

rspec ./spec/unit/lib/hooks/handlers/error_spec.rb:12 # Hooks::Plugins::Handlers::Error#initialize creates an error with custom status

Stopped processing SimpleCov as a previous error not related to SimpleCov has been detected
Coverage report generated for RSpec to /home/runner/work/hooks/hooks/coverage.
Line Coverage: 100.0% (868 / 868)

✅ Total Coverage: 100.0 (required: 100%)

Please resolve these errors in the unit tests. When finished, make sure to run script/integration and script/acceptance to make sure everything still works.

Copy link
Contributor Author

Copilot AI commented Jun 16, 2025

@copilot everything looks really good and works great on my machine running Ruby 3.4.4 (the latest version of Ruby). However, I see that several older versions are failing when running script/test...

Fixed the test compatibility issue with different Ruby versions. The test was expecting Ruby 3.4.4's hash formatting but older versions use different syntax. Updated the test expectation to use a regex pattern that works across all Ruby versions. All tests now pass. (2760d30)

Copilot finished work on behalf of GrantBirki June 16, 2025 23:39
@GrantBirki GrantBirki added the enhancement New feature or request label Jun 17, 2025
@GrantBirki GrantBirki merged commit 80d59a7 into main Jun 17, 2025
22 checks passed
@GrantBirki GrantBirki deleted the copilot/fix-49 branch June 17, 2025 00:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

task: allow handler subclasses to call Grape's error! method
2 participants