Catch scc request timeout exception #63
Conversation
This commit adds proper timeout exception treatment on the method `RMT::WizardSCCPage#scc_credentials_valid?`: * Before: an "Internal error" message would be shown if a request timed out when contacting SCC to validate the credentials. * Now: a popup dialog will be shown, giving the user the choice to retry the request or cancel the operation. Additionally, the request has been enriched with a proper "user agent" to allow better debugging with SCC.
| valid_credentials = nil | ||
| while valid_credentials.nil? | ||
| begin | ||
| res = Net::HTTP.start(uri.host, uri.port, use_ssl: true) { |http| http.request(req) } |
There was a problem hiding this comment.
Just a minor suggestion:
| res = Net::HTTP.start(uri.host, uri.port, use_ssl: true) { |http| http.request(req) } | |
| res = Net::HTTP.start(uri.host, uri.port, use_ssl: uri.is_a?(URI::HTTPS)) { |http| http.request(req) } |
This is more robust as it will still work after you change the URL above to plain HTTP.
There was a problem hiding this comment.
It's a nice enhancement, but since the URL is hard-coded, it will not be very helpful.
I wonder if it would be valuable for test and development purposes to have a way to configure the SCC URL (e.g.: env var), then this change would be very welcome. I had to manually change both the URL and the use_ssl flag whenever I tested it locally.
src/lib/rmt/wizard_scc_page.rb
Outdated
| @@ -26,6 +26,8 @@ module RMT; end | |||
| class RMT::WizardSCCPage < Yast::Client | |||
| include ::UI::EventDispatcher | |||
|
|
|||
| YAST_RMT_USER_AGENT = 'yast2-rmt'.freeze | |||
There was a problem hiding this comment.
What about adding some version number to the agent string? Standard web browsers do that as well.
Non-working pseudocode:
YAST_RMT_USER_AGENT = "yast2-rmt/#{RMT::VERSION}".freezeIn theory this could allow to handle some corner cases or bugs in SMT on the server size. Or if the SCC changes the API it could fallback to old API for old versions...
Ideally this should not be needed but it would be nice to have this possibility as the last resort...
There was a problem hiding this comment.
I created the constant module RMT::VERSION, added it to the user agent, and created a unit test to ensure it always matches the package version. Please check commit 89e919c.
lcaparroz
force-pushed
the
catch-scc-request-timeout-exception
branch
from
8c5cfa7
to
7d5fb82
Compare
To enhance the 'User-Agent' used in the request headers, this commit adds the module constant `RMT::Version`, which must always match the package version specified in 'package/yast2-rmt.spec'. Additionally, a unit test was created to ensure the versions always match each other; if there is a version bump in packge spec file, the RSpec tests will break if 'src/lib/rmt.rb' is not updated accordingly.
lcaparroz
force-pushed
the
catch-scc-request-timeout-exception
branch
from
7d5fb82
to
89e919c
Compare
| 2. Run the Docker container with access to the localhost network with the chosen distribution and version: | ||
|
|
||
| ```shell | ||
| docker run --network host -v "$(pwd):/usr/src/app" -w "/usr/src/app" -it registry.opensuse.org/yast/sle-15/sp6/containers/yast-ruby sh |
There was a problem hiding this comment.
Thank you very much Luis! This is awesome!
There was a problem hiding this comment.
lgtm 👍
How I reviewed this pull request:
- Checked the awesome testing docs!
- Run yast2-rmt against a local SCC:
- Checked if the timeout works
- Checked that everything works when no timeout occurs
- Checked the suggestions and also the tests
As always, if you think I missed something I should test, please let me know!🚀
Problem
When the SCC API times out during credentials validation, the module would break with an "Internal error" message displaying details about the Ruby exception.
Solution
Proper exception treatment was implemented, and now, a UI popup is displayed to the user, giving them the choice to either retry the validation or cancel the operation.
Testing
spec/rmt/wizard_scc_page_spec.rb.READMEinstructions for a Docker setup, running the module against a localhost server to simulate a timeout.Screenshots
1. [Existing] Initial screen when running
yast2 rmt2. [Existing] Screen after pressing
Nextto verify SCC Credentials and the request is taking too long to complete3. [New] Popup dialog shown to the user when the request times out
The new UI was added on this step. This is the only change to UI.
4. [Existing] Screen if the user chooses to cancel the operation