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

Implement transparent client pattern and middleware error handling #147

Merged
merged 16 commits into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all 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
21 changes: 0 additions & 21 deletions lib/seam/base_client.rb

This file was deleted.

4 changes: 4 additions & 0 deletions lib/seam/deep_hash_accessor.rb
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ def initialize(data)
create_accessor_methods
end

def [](key)
instance_variable_get(:"@#{key}")
end

private

def create_accessor_methods
Expand Down
18 changes: 5 additions & 13 deletions lib/seam/helpers/action_attempt.rb
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,11 @@ module Seam
module Helpers
module ActionAttempt
def self.decide_and_wait(action_attempt, client, wait_for_action_attempt)
wait_decision = wait_for_action_attempt.nil? ? client.defaults.wait_for_action_attempt : wait_for_action_attempt

if wait_decision == true
if wait_for_action_attempt == true
return wait_until_finished(action_attempt, client)
elsif wait_decision.is_a?(Hash)
return wait_until_finished(action_attempt, client, timeout: wait_decision[:timeout],
polling_interval: wait_decision[:polling_interval])
elsif wait_for_action_attempt.is_a?(Hash)
return wait_until_finished(action_attempt, client, timeout: wait_for_action_attempt[:timeout],
polling_interval: wait_for_action_attempt[:polling_interval])
end

action_attempt
Expand All @@ -37,13 +35,7 @@ def self.wait_until_finished(action_attempt, client, timeout: nil, polling_inter
end

def self.update_action_attempt(action_attempt, client)
response = client.request_seam(
:post,
"/action_attempts/get",
body: {
action_attempt_id: action_attempt.action_attempt_id
}
)
response = client.get("/action_attempts/get", {action_attempt_id: action_attempt.action_attempt_id})

action_attempt.update_from_response(response.body["action_attempt"])
action_attempt
Expand Down
12 changes: 1 addition & 11 deletions lib/seam/http_multi_workspace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ def lts_version
end

def workspaces
@workspaces ||= WorkspacesProxy.new(Seam::Clients::Workspaces.new(self))
@workspaces ||= WorkspacesProxy.new(Seam::Clients::Workspaces.new(client: @client, defaults: @defaults))
end

def self.from_personal_access_token(personal_access_token, endpoint: nil, wait_for_action_attempt: true, faraday_options: {}, faraday_retry_options: {})
Expand All @@ -41,16 +41,6 @@ def self.from_personal_access_token(personal_access_token, endpoint: nil, wait_f
faraday_retry_options: faraday_retry_options
)
end

def request_seam_object(method, path, klass, inner_object, config = {})
response = Seam::Http::Request.request_seam(@client, @endpoint, method, path, config)
data = response.body[inner_object]
klass.load_from_response(data, self)
end

def request_seam(method, path, config = {})
Seam::Http::Request.request_seam(@client, @endpoint, method, path, config)
end
end

class WorkspacesProxy
Expand Down
13 changes: 2 additions & 11 deletions lib/seam/http_single_workspace.rb
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,10 @@ def initialize(client: nil, api_key: nil, personal_access_token: nil, workspace_
@endpoint = options[:endpoint]
@auth_headers = options[:auth_headers]
@defaults = Seam::DeepHashAccessor.new({"wait_for_action_attempt" => wait_for_action_attempt})

@client = client || Seam::Http::Request.create_faraday_client(@endpoint, @auth_headers, faraday_options,
faraday_retry_options)

initialize_routes(client: @client, defaults: @defaults)
Copy link
Contributor

Choose a reason for hiding this comment

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

What does this do? Why is it defined as a private function but called here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

initialize_routes is an internal method to set up route clients by making available @client and @defaults for them. SingleWorkspace can access private initialize_routes because Routes module gets included in the class. While the method will exist on instances, being private it can only be called internally (inside the class/module methods) and not from outside code, unlike public methods like 'devices', 'access_codes' etc. which can be called by anyone using the instance.

Copy link
Contributor

Choose a reason for hiding this comment

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

Why is this method needed though vs just putting the code inline? I think I must be missing something obvious.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By including it inline, you mean to include the code bellow in the SingleWorkspace class?

def access_codes
  @access_codes ||= Seam::Clients::AccessCodes.new(client: @client, defaults: @defaults)
end

...

We could have done that, but we decided to generate route-related stuff, so I placed the clients under the Routes module (which is generated) so that we won't need to manually update the SDK when new namespaces are introduced in the Seam API. Let me know if you have a better approach in mind.

Copy link
Contributor

Choose a reason for hiding this comment

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

I was wondering why not to just do

      @client = client
      @defaults = defaults

But now I understand what this is doing. Basically this module only works if mixed into classes that define @client and @defaults. And this method is helping clarify that requirement.

I wonder if there is a more standard pattern to handle this.
I think it may be what this question is asking about https://stackoverflow.com/questions/12586051/initializing-instance-variables-in-mixins

end

def lts_version
Expand All @@ -36,16 +37,6 @@ def self.from_personal_access_token(personal_access_token, workspace_id, endpoin
new(personal_access_token: personal_access_token, workspace_id: workspace_id, endpoint: endpoint,
wait_for_action_attempt: wait_for_action_attempt, faraday_options: faraday_options, faraday_retry_options: faraday_retry_options)
end

def request_seam_object(method, path, klass, inner_object, config = {})
response = Seam::Http::Request.request_seam(@client, @endpoint, method, path, config)
data = response.body[inner_object]
klass.load_from_response(data, self)
end

def request_seam(method, path, config = {})
Seam::Http::Request.request_seam(@client, @endpoint, method, path, config)
end
end
end
end
84 changes: 49 additions & 35 deletions lib/seam/request.rb
Original file line number Diff line number Diff line change
Expand Up @@ -23,42 +23,9 @@ def self.create_faraday_client(endpoint, auth_headers, faraday_options = {}, far

Faraday.new(options) do |builder|
builder.request :json
builder.request :retry, faraday_retry_options
builder.response :json
end
end

def self.handle_error_response(response, _method, _path)
status_code = response.status
request_id = response.headers["seam-request-id"]

raise Http::UnauthorizedError.new(request_id) if status_code == 401

error = response.body.is_a?(Hash) ? response.body["error"] || {} : {}
error_type = error["type"] || "unknown_error"
error_message = error["message"] || "Unknown error"
error_details = {
type: error_type,
message: error_message,
data: error["data"]
}

if error_type == "invalid_input"
error_details["validation_errors"] = error["validation_errors"]
raise Http::InvalidInputError.new(error_details, status_code, request_id)
end

raise Http::ApiError.new(error_details, status_code, request_id)
end

def self.request_seam(client, endpoint, method, path, config = {})
url = "#{endpoint}#{path}"
response = client.run_request(method, url, config[:body], config[:headers])

if response.success?
response
else
handle_error_response(response, method, path)
builder.use ResponseMiddleware
builder.request :retry, faraday_retry_options
end
end

Expand All @@ -72,6 +39,53 @@ def self.default_headers
}
end

class ResponseMiddleware < Faraday::Response::RaiseError
def on_complete(env)
return if env.success?

status_code = env.status
request_id = env.response_headers["seam-request-id"]

raise Http::UnauthorizedError.new(request_id) if status_code == 401

if seam_api_error_response?(env)
body = JSON.parse(env.body)
error = body["error"]
error_details = {
type: error["type"] || "unknown_error",
message: error["message"] || "Unknown error",
data: error["data"]
}

if error["type"] == "invalid_input"
error_details["validation_errors"] = error["validation_errors"]
raise Http::InvalidInputError.new(error_details, status_code, request_id)
end

raise Http::ApiError.new(error_details, status_code, request_id)
end

super
end

def seam_api_error_response?(env)
return false unless env.response_headers

content_type = env.response_headers["Content-Type"]
return false unless content_type&.start_with?("application/json")

begin
body = JSON.parse(env.body)
return false unless body.is_a?(Hash) && body["error"].is_a?(Hash)

error = body["error"]
error["type"].is_a?(String) && error["message"].is_a?(String)
rescue JSON::ParserError
false
end
end
end

def self.deep_merge(hash1, hash2)
result = hash1.dup
hash2.each do |key, value|
Expand Down
89 changes: 29 additions & 60 deletions lib/seam/routes/clients/access_codes.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

17 changes: 9 additions & 8 deletions lib/seam/routes/clients/access_codes_simulate.rb

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading