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.

Sign up for GitHub

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

Jump to bottom

Use a URL that won't respond for composer private registry timeout test #6776

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft

Use a URL that won't respond for composer private registry timeout test #6776

wants to merge 1 commit into from

Conversation

jeffwidman
Copy link
Member

@jeffwidman jeffwidman commented Mar 3, 2023 •
edited
Loading

https://composer.dependabot.com no longer has a DNS record now that Dependabot was acquired by GitHub. And we can't just stub the URL because the call is happening within the composer subprocess, not in our ruby process.

This helpful StackOverflow pointed out that example.com is owned by IANA so should be safe, and port 81 is never expected to respond: https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error

Alternatively, we could use a non-routable IP but that may return different errors depending on the topology of the LAN where it's running.

This is a stacked PR on top of #6434

@jeffwidman jeffwidman mentioned this pull request Mar 3, 2023
Merged
Base automatically changed from re-enable-composer-timeout-test to main March 3, 2023 05:45
@jeffwidman jeffwidman changed the title Use a URL that won't respond Use a URL that won't respond for composer private registry timeout test Mar 3, 2023
@jeffwidman
Use a URL that won't respond
a6771c0 
`https://composer.dependabot.com` no longer has a DNS record now that
Dependabot was acquired by GitHub. And we can't just stub the URL
because the call is happening within the `composer` subprocess, not in
our ruby process.

This helpful StackOverflow pointed out that example.com is owned by IANA
so should be safe, and port 81 is never expected to respond:
https://stackoverflow.com/questions/100841/artificially-create-a-connection-timeout-error

Alternatively, we could use a non-routable IP but that may return
different errors depending on the topology of the LAN where it's running.
@jeffwidman jeffwidman force-pushed the use-example.com-to-hang branch from b63fb0e to a6771c0 Compare March 3, 2023 05:48
@jeffwidman
Copy link
Member Author 

Failures:

  1) Dependabot::Composer::UpdateChecker::VersionResolver latest_resolvable_version with an unreachable private registry raises a Dependabot::PrivateSourceTimedOut error
     Failure/Error:
       expect { resolver.latest_resolvable_version }.
         to raise_error(Dependabot::PrivateSourceTimedOut) do |error|
           expect(error.source).to eq("https://example.com/")
         end

       expected Dependabot::PrivateSourceTimedOut, got #<Dependabot::SharedHelpers::HelperSubprocessFailed: curl error 7 while downloading https://example.com:81/packages.json: Failed to connect to example.com port 81: Connection timed out> with backtrace:
         # /home/dependabot/common/lib/dependabot/shared_helpers.rb:123:in `run_helper_subprocess'
         # ./lib/dependabot/composer/update_checker/version_resolver.rb:133:in `block in run_update_checker'
         # /home/dependabot/common/lib/dependabot/shared_helpers.rb:169:in `with_git_configured'
         # ./lib/dependabot/composer/update_checker/version_resolver.rb:132:in `run_update_checker'
         # ./lib/dependabot/composer/update_checker/version_resolver.rb:83:in `block in fetch_latest_resolvable_version_string'
         # /home/dependabot/common/lib/dependabot/shared_helpers.rb:49:in `block in in_a_temporary_directory'
         # /home/dependabot/common/lib/dependabot/shared_helpers.rb:49:in `chdir'
         # /home/dependabot/common/lib/dependabot/shared_helpers.rb:49:in `in_a_temporary_directory'
         # ./lib/dependabot/composer/update_checker/version_resolver.rb:81:in `fetch_latest_resolvable_version_string'
         # ./lib/dependabot/composer/update_checker/version_resolver.rb:66:in `fetch_latest_resolvable_version'
         # ./lib/dependabot/composer/update_checker/version_resolver.rb:56:in `latest_resolvable_version'
         # ./spec/dependabot/composer/update_checker/version_resolver_spec.rb:288:in `block (5 levels) in <top (required)>'
         # ./spec/dependabot/composer/update_checker/version_resolver_spec.rb:288:in `block (4 levels) in <top (required)>'
         # /home/dependabot/common/spec/spec_helper.rb:63:in `block (2 levels) in <top (required)>'
     # ./spec/dependabot/composer/update_checker/version_resolver_spec.rb:288:in `block (4 levels) in <top (required)>'
     # /home/dependabot/common/spec/spec_helper.rb:63:in `block (2 levels) in <top (required)>'

So the test indicates the code is broken.

@jeffwidman
Copy link
Member Author

This will be much easier to work on once composer v1 support is dropped, as the error messages will become more deterministic:

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers
No reviews
Assignees

@jeffwidman jeffwidman

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
1 participant
@jeffwidman