Skip to content
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

Monkey patch gruf interceptor #17

Merged
merged 10 commits into from
Oct 31, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions Gemfile.lock
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ PATH
specs:
grpc-rest (0.1.21)
grpc
gruf
rails (>= 6.0)

GEM
Expand Down
15 changes: 14 additions & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,22 @@ service MyService {

## Gruf Interceptors

grpc-rest supports [gruf](https://github.com/bigcommerce/gruf) Interceptors. As long as you're not using a custom interceptor
grpc-rest supports [gruf](https://github.com/bigcommerce/gruf) Interceptors through a custom `GrpcRest::BaseInterceptor` class. As long as you're not using a custom interceptor
registry, your interceptors will be called normally around the controller.

```ruby
module Interceptors
# Interceptor for catching errors from controllers
class ErrorInterceptor < GrpcRest::BaseInterceptor

def call
# Your code here
end

end
end
```

## To Do

* Replace Go implementation with Ruby (+ executable)
Expand Down
1 change: 1 addition & 0 deletions grpc-rest.gemspec
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ Gem::Specification.new do |spec|

spec.add_runtime_dependency('grpc')
spec.add_runtime_dependency('rails', '>= 6.0')
spec.add_runtime_dependency('gruf')
Copy link
Member

Choose a reason for hiding this comment

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

gruf doesn't need to be a runtime dependency. I forgot that I didn't want to enforce this.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ah okay, sounds good. Just wondering, what is the rationale for not including it? I assumed that since we were explicitly using Gruf::Interceptors::ServerInterceptor in the code we'd need it as a dependency.

Copy link
Member

Choose a reason for hiding this comment

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

That's only in the send_gruf_request method, which only gets used if Gruf is loaded.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Do you think adding the ErrorInterceptor you made by default into grpc-rest makes sense? Or is it specific to the publication stuff?

I feel like it might be too domain-specific to include in grpc-rest, but I am not sure. Maybe it would be best to include the error interceptor as an example in README.md? What do you think?

Copy link
Member

Choose a reason for hiding this comment

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

Nah, I think the example we have is fine. I forgot it had some ActiveRecord etc stuff in there.


spec.add_development_dependency('rspec-rails')
spec.add_development_dependency('gruf')
Expand Down
1 change: 1 addition & 0 deletions lib/grpc_rest.rb
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
require 'google/protobuf/well_known_types'
require 'grpc'
require 'grpc/core/status_codes'
require 'interceptors/base_interceptor'
Copy link
Member

Choose a reason for hiding this comment

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

You can remove this line - you can add to the readme that if you want to make an interceptor you have to require this before you do it.


module GrpcRest
class << self
Expand Down
46 changes: 46 additions & 0 deletions lib/interceptors/base_interceptor.rb
Original file line number Diff line number Diff line change
@@ -0,0 +1,46 @@
# frozen_string_literal: true

require 'gruf'

module GrpcRest
# This is a monkey-patch that fixes an issue we were having with using the fail! method in
# interceptors where an active call was not instantiated yet.
# Basically, we overloaded this function: https://github.com/bigcommerce/gruf/blob/main/lib/gruf/errors/helpers.rb#L34
class BaseInterceptor < ::Gruf::Interceptors::ServerInterceptor
def fail!(error_code, _app_code = 500, message = 'unkown error', metadata = {})
raise grpc_error(error_code, message.to_s, metadata)
end

private

# Ported from https://github.com/flipp-oss/grpc-rest/blob/main/lib/grpc_rest.rb#L142
def grpc_error(error_code, message, metadata)
case error_code
when :ok
GRPC::Ok.new(message, metadata)
when 499
GRPC::Cancelled.new(message, metadata)
when :bad_request, :invalid_argument
GRPC::InvalidArgument.new(message, metadata)
when :gateway_timeout
GRPC::DeadlineExceeded.new(message, metadata)
when :not_found
GRPC::NotFound.new(message, metadata)
when :conflict
GRPC::AlreadyExists.new(message, metadata)
when :forbidden
GRPC::PermissionDenied.new(message, metadata)
when :unauthorized
GRPC::Unauthenticated.new(message, metadata)
when :too_many_requests
GRPC::ResourceExhausted.new(message, metadata)
when :not_implemented
GRPC::Unimplemented.new(message, metadata)
when :service_unavailable
GRPC::Unavailable.new(message, metadata)
else
GRPC::Internal.new(message, metadata)
end
end
end
end
Loading