From 43ddfb9f6e2fc9778b2e9435a1b5fbbc03f1f4c1 Mon Sep 17 00:00:00 2001 From: David Chandek-Stark Date: Mon, 11 Mar 2024 20:03:02 -0400 Subject: [PATCH 1/3] Version 1.10.0 - Handles server errors by HTTP code - Drops Ruby 2.7 testing, adds Ruby 3.2 and 3.3 testing --- .dockerignore | 1 + .github/workflows/ruby.yml | 2 +- Dockerfile | 8 ++++---- VERSION | 2 +- lib/ezid/error.rb | 2 ++ lib/ezid/requests/request.rb | 12 ++++++++++-- lib/ezid/responses/response.rb | 2 -- spec/unit/client_spec.rb | 14 ++------------ 8 files changed, 21 insertions(+), 22 deletions(-) diff --git a/.dockerignore b/.dockerignore index ff2c781..d79099a 100644 --- a/.dockerignore +++ b/.dockerignore @@ -2,6 +2,7 @@ .bundle .config .dockerignore +.git .github .gitignore Dockerfile diff --git a/.github/workflows/ruby.yml b/.github/workflows/ruby.yml index e0d2c5d..6dbc176 100644 --- a/.github/workflows/ruby.yml +++ b/.github/workflows/ruby.yml @@ -19,7 +19,7 @@ jobs: runs-on: ubuntu-latest strategy: matrix: - ruby-version: ['2.7', '3.0', '3.1'] + ruby-version: ['3.0', '3.1', '3.2', '3.3'] steps: - uses: actions/checkout@v2 diff --git a/Dockerfile b/Dockerfile index 2f1b6fa..26cfadc 100644 --- a/Dockerfile +++ b/Dockerfile @@ -4,10 +4,10 @@ FROM ruby:${ruby_version} SHELL ["/bin/bash", "-c"] -RUN gem install bundler -v '~>2.0' - WORKDIR /app -COPY . . +COPY VERSION Gemfile ezid-client.gemspec ./ -RUN bundle install +RUN gem install bundler -v '~>2.0' && bundle install + +COPY . . diff --git a/VERSION b/VERSION index d615fd0..81c871d 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.9.4 +1.10.0 diff --git a/lib/ezid/error.rb b/lib/ezid/error.rb index 323e22a..ec5f883 100644 --- a/lib/ezid/error.rb +++ b/lib/ezid/error.rb @@ -10,4 +10,6 @@ class NotAllowedError < Error; end class DeletionError < Error; end class UnexpectedResponseError < Error; end + + class ServerError < Error; end end diff --git a/lib/ezid/requests/request.rb b/lib/ezid/requests/request.rb index 21b4de5..ac79c2f 100644 --- a/lib/ezid/requests/request.rb +++ b/lib/ezid/requests/request.rb @@ -22,6 +22,8 @@ class Request < SimpleDelegator POST = Net::HTTP::Post DELETE = Net::HTTP::Delete + RETRIABLE_SERVER_ERRORS = %w[500 502 503 504].freeze + class << self attr_accessor :http_method, :path, :response_class @@ -51,8 +53,14 @@ def initialize(client, *args) def execute retries = 0 begin - response_class.new(get_response_for_request) - rescue Net::HTTPServerException, UnexpectedResponseError => e + http_response = get_response_for_request + + if RETRIABLE_SERVER_ERRORS.include? http_response.code + raise ServerError, "#{http_response.code} #{http_response.msg}" + end + + response_class.new(http_response) + rescue ServerError, UnexpectedResponseError => e if retries < 2 sleep client.config.retry_interval retries += 1 diff --git a/lib/ezid/responses/response.rb b/lib/ezid/responses/response.rb index 4c8f833..6843515 100644 --- a/lib/ezid/responses/response.rb +++ b/lib/ezid/responses/response.rb @@ -15,8 +15,6 @@ class Response < SimpleDelegator ERROR = "error".freeze def initialize(http_response) - http_response.value # raises Net::HTTPServerException - super unless status_line =~ /^(#{SUCCESS}|#{ERROR}): / diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb index bc7fde1..2fe2821 100644 --- a/spec/unit/client_spec.rb +++ b/spec/unit/client_spec.rb @@ -179,16 +179,6 @@ module Ezid allow(GetIdentifierMetadataRequest).to receive(:execute).with(subject, "invalid") { stub_response } end - describe "HTTP error response" do - before do - allow(http_response).to receive(:value).and_raise(Net::HTTPServerException.new('Barf!', double)) - end - - it "raises an exception" do - expect { subject.get_identifier_metadata("invalid") }.to raise_error(Net::HTTPServerException) - end - end - describe "EZID API error response" do let(:body) { "error: bad request - no such identifier" } @@ -223,8 +213,8 @@ module Ezid allow(GetIdentifierMetadataResponse).to receive(:new).and_raise(exception) end - describe "HTTP error" do - let(:exception) { Net::HTTPServerException.new('Barf!', double) } + describe "retriable server error" do + let(:exception) { ServerError.new('502 Bad Gateway') } it "retries twice" do begin From 963e8b5101ef91ef5e65782ced5313b560323516 Mon Sep 17 00:00:00 2001 From: David Chandek-Stark Date: Mon, 11 Mar 2024 20:32:16 -0400 Subject: [PATCH 2/3] Adds logging of retriable errors --- lib/ezid/requests/request.rb | 7 ++++++- spec/unit/client_spec.rb | 1 + 2 files changed, 7 insertions(+), 1 deletion(-) diff --git a/lib/ezid/requests/request.rb b/lib/ezid/requests/request.rb index ac79c2f..27d3a74 100644 --- a/lib/ezid/requests/request.rb +++ b/lib/ezid/requests/request.rb @@ -39,7 +39,7 @@ def short_name end attr_reader :client - def_delegators :client, :connection, :user, :password, :session + def_delegators :client, :connection, :user, :password, :session, :logger # @param client [Ezid::Client] the client def initialize(client, *args) @@ -52,6 +52,7 @@ def initialize(client, *args) # @return [Ezid::Response] the response def execute retries = 0 + begin http_response = get_response_for_request @@ -60,10 +61,14 @@ def execute end response_class.new(http_response) + rescue ServerError, UnexpectedResponseError => e if retries < 2 + logger.error "EZID error: #{e}" + sleep client.config.retry_interval retries += 1 + retry else raise diff --git a/spec/unit/client_spec.rb b/spec/unit/client_spec.rb index 2fe2821..3d93016 100644 --- a/spec/unit/client_spec.rb +++ b/spec/unit/client_spec.rb @@ -210,6 +210,7 @@ module Ezid describe "retrying on certain errors" do before do + allow(subject.config).to receive(:retry_interval) { 1 } allow(GetIdentifierMetadataResponse).to receive(:new).and_raise(exception) end From 7b5b3b395818dadff91b818b489f1d25bede64ec Mon Sep 17 00:00:00 2001 From: David Chandek-Stark Date: Tue, 12 Mar 2024 17:18:05 -0400 Subject: [PATCH 3/3] More logging --- lib/ezid/requests/request.rb | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/lib/ezid/requests/request.rb b/lib/ezid/requests/request.rb index 27d3a74..a95d74f 100644 --- a/lib/ezid/requests/request.rb +++ b/lib/ezid/requests/request.rb @@ -24,6 +24,8 @@ class Request < SimpleDelegator RETRIABLE_SERVER_ERRORS = %w[500 502 503 504].freeze + RETRIES = ENV.fetch('EZID_REQUEST_RETRIES', '2').to_i + class << self attr_accessor :http_method, :path, :response_class @@ -39,7 +41,7 @@ def short_name end attr_reader :client - def_delegators :client, :connection, :user, :password, :session, :logger + def_delegators :client, :connection, :user, :password, :session, :logger, :config # @param client [Ezid::Client] the client def initialize(client, *args) @@ -63,11 +65,12 @@ def execute response_class.new(http_response) rescue ServerError, UnexpectedResponseError => e - if retries < 2 + if retries < RETRIES logger.error "EZID error: #{e}" - sleep client.config.retry_interval retries += 1 + logger.info "Retry (#{retries} of #{RETRIES}) of #{short_name} #{path} in #{config.retry_interval} seconds ..." + sleep config.retry_interval retry else @@ -98,6 +101,10 @@ def response_class # @return [String] the query string def query; end + def short_name + self.class.short_name + end + def authentication_required? true end