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

Added option to retry the block given #428

Merged
merged 5 commits into from
Mar 13, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
34 changes: 13 additions & 21 deletions src/lib/registration/connect_helpers.rb
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,6 @@ class ConnectHelpers
def self.catch_registration_errors(message_prefix: "",
show_update_hint: false,
silent_reg_code_mismatch: false,
retry_block: false,
&block)
# import the SSL certificate just once to avoid an infinite loop
certificate_imported = false
Expand All @@ -83,15 +82,9 @@ def self.catch_registration_errors(message_prefix: "",
log.error "Timeout error: #{e.message}"
# FIXME: to not break existing translation, this typo should be fixed
# later after SP2: time -> timed
if retry_block
retry if report_error_and_retry?(message_prefix + _("Connection time out."),
_("Make sure that the registration server is reachable and\n" \
"the connection is reliable."))
else
report_error(message_prefix + _("Connection time out.") + "\n",
_("Make sure that the registration server is reachable and\n" \
"the connection is reliable."))
end
retry if report_error_and_retry?(message_prefix + _("Connection time out."),
_("Make sure that the registration server is reachable and\n" \
"the connection is reliable."))

false
rescue SUSE::Connect::ApiError => e
Expand Down Expand Up @@ -148,12 +141,7 @@ def self.catch_registration_errors(message_prefix: "",
log.error "JSON parse error"
# update the message when an old SMT server is found
check_smt_api(e.message)
msg = message_prefix + _("Cannot parse the data from server.")
if retry_block
retry if report_error_and_retry?(msg, e.message)
else
report_error(msg, e.message)
end
details_error(message_prefix + _("Cannot parse the data from server."), e.message)
Copy link
Contributor Author

Choose a reason for hiding this comment

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

while Yast::Report returned nil, details_error reports :ok so, added the return value explicitly as we have in the other rescued exceptions

rescue StandardError => e
log.error("SCC registration failed: #{e.class}: #{e}, #{e.backtrace}")
Yast::Report.Error(
Expand All @@ -168,13 +156,17 @@ def self.report_error(msg, error_message)
Yast::Report.Error(error_with_details(msg, error_message))
end

def self.retry_error(msg, error_message)
buttons = { retry: _("Retry"), cancel: _("Cancel") }
Yast2::Popup.show(msg, details: error_message, headline: :error, buttons: buttons)
def self.details_error(msg, error_message, retry_button: false)
if Yast::Mode.autoinst || Yast::Mode.autoupgrade
teclator marked this conversation as resolved.
Show resolved Hide resolved
report_error(msg, error_message)
else
buttons = retry_button ? { retry: _("Retry"), cancel: _("Cancel") } : :ok
teclator marked this conversation as resolved.
Show resolved Hide resolved
Yast2::Popup.show(msg, details: error_message, headline: :error, buttons: buttons)
end
end

def self.report_error_and_retry?(msg, details_message)
retry_error(msg, details_message) == :retry
details_error(msg, details_message, retry_button: true) == :retry
end

# Report a pkg-bindings error. Display a message with error details from
Expand Down Expand Up @@ -358,7 +350,7 @@ def self.add_update_hint(error_msg)
end

private_class_method :report_error, :error_with_details, :import_ssl_certificate,
:report_ssl_error, :check_smt_api, :handle_network_error, :retry_error,
:report_ssl_error, :check_smt_api, :handle_network_error, :details_error,
:report_error_and_retry?
end
end
3 changes: 2 additions & 1 deletion src/lib/registration/registration_ui.rb
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,8 @@ def initialize(registration)
# items: boolean (true on success), remote service (or nil)
def register_system_and_base_product
product_service = nil
error_options = { message_prefix: "The registration failed.", retry_block: true }
# TRANSLATORS: Popup error message prefix
error_options = { message_prefix: _("Registration failed.") }
teclator marked this conversation as resolved.
Show resolved Hide resolved

success = ConnectHelpers.catch_registration_errors(error_options) do
register_system if !Registration.is_registered?
Expand Down
53 changes: 26 additions & 27 deletions test/connect_helpers_spec.rb
Original file line number Diff line number Diff line change
Expand Up @@ -51,13 +51,20 @@ def api_error(code: 400, headers: {}, body: {})
_("Make sure that the registration server is reachable and\n" \
"the connection is reliable.")
end
let(:autoinst) { false }

before do
allow(Yast::Mode).to receive(:autoinst).and_return(:autoinst?)
end

context "and in AutoYaST mode" do
let(:autoinst?) { true }

context "and retry_block disabled" do
it "reports an error with details on timeout" do
expect(subject).to receive(:wrap_text).with("Details: #{details}", screen_width - 4)
.and_return("Details wrapped text")
expect(Yast::Report).to receive(:Error).with(
"Registration: " + _("Connection time out.") + "\n\n\nDetails wrapped text"
"Registration: " + _("Connection time out.") + "\n\nDetails wrapped text"
)

helpers.catch_registration_errors(message_prefix: "Registration: ") do
Expand All @@ -66,12 +73,12 @@ def api_error(code: 400, headers: {}, body: {})
end
end

context "and with retry_block enabled" do
context "and in a common installation" do
it "shows an error dialog with a retry and details button" do
expect(helpers).to receive(:report_error_and_retry?)
.with("Registration: " + _("Connection time out."), details)

helpers.catch_registration_errors(message_prefix: "Registration: ", retry_block: true) do
helpers.catch_registration_errors(message_prefix: "Registration: ") do
raise Timeout::Error
end
end
Expand All @@ -80,7 +87,7 @@ def api_error(code: 400, headers: {}, body: {})
expect(helpers).to receive(:report_error_and_retry?).twice
.with("Registration: " + _("Connection time out."), details).and_return(true, false)

helpers.catch_registration_errors(message_prefix: "Registration: ", retry_block: true) do
helpers.catch_registration_errors(message_prefix: "Registration: ") do
raise Timeout::Error
end
end
Expand Down Expand Up @@ -136,33 +143,25 @@ def api_error(code: 400, headers: {}, body: {})
end

context "JSON parse error is received" do
include_examples "old registration server", JSON::ParserError.new("error message")
let(:autoinst?) { true }

let(:details) { "server error response details" }
let(:exception) { JSON::ParserError.new(details) }

context "and with retry_block enabled" do
before do
allow(Registration::UrlHelpers).to receive(:registration_url)
.and_return(SUSE::Connect::YaST::DEFAULT_URL)
end
before do
allow(Yast::Mode).to receive(:autoinst).and_return(autoinst?)
allow(Registration::UrlHelpers).to receive(:registration_url)
.and_return(SUSE::Connect::YaST::DEFAULT_URL)
end

it "shows an error dialog with a retry and details button" do
expect(helpers).to receive(:report_error_and_retry?)
.with("Registration: " + _("Cannot parse the data from server."), details)
include_examples "old registration server", JSON::ParserError.new("error message")

helpers.catch_registration_errors(message_prefix: "Registration: ", retry_block: true) do
raise exception
end
end
context "and in interactive installation" do
let(:autoinst?) { false }

it "retries the given block if selected by the user" do
expect(helpers).to receive(:report_error_and_retry?).twice
.with("Registration: " + _("Cannot parse the data from server."), details)
.and_return(true, false)
it "hiddes the error details in a details button" do
expect(helpers).to receive(:details_error)
.with("Registration: " + _("Cannot parse the data from server."), "error message")

helpers.catch_registration_errors(message_prefix: "Registration: ", retry_block: true) do
raise exception
helpers.catch_registration_errors(message_prefix: "Registration: ") do
raise JSON::ParserError, "error message"
end
end
end
Expand Down