Skip to content
This repository was archived by the owner on Sep 30, 2024. It is now read-only.

cmd: frontend: add note that clarifies that your code host needs to be accessible over the network to test connection button#62977

Merged
ggilmore merged 1 commit into
mainfrom
05-29-wip_try_adding_note_to_repository_check_page_about_publically_accessible_urls
Jun 3, 2024
Merged

cmd: frontend: add note that clarifies that your code host needs to be accessible over the network to test connection button#62977
ggilmore merged 1 commit into
mainfrom
05-29-wip_try_adding_note_to_repository_check_page_about_publically_accessible_urls

Conversation

@ggilmore

@ggilmore ggilmore commented May 30, 2024 •
edited
Loading

Copy link
Copy Markdown
Contributor

Closes https://linear.app/sourcegraph/issue/SRC-359/

In the case where a user clicks on the "test connection" button and the:

  1. The request never completed within 15 seconds
  2. The request failed with a net.DNSerror.isNotFound error

One pretty good guess is that the code host is not actually accessible to Sourcegraph over the network. This PR tweaks the error message to add this hint + a link to our troubleshooting documentation. (See sourcegraph/docs#374)

Test plan

Screenshots of this:

Screenshot 2024-05-30 at 12.43.08 PM.png

Screenshot 2024-05-30 at 12.42.29 PM.png

Recordings:

  1. Testing with a bad domain name
🎥 Video uploaded on Graphite:
  1. Testing with a timeout (the IP address will never resolve)
🎥 Video uploaded on Graphite:
  1. Updating an existing code host connection with a bad address
🎥 Video uploaded on Graphite:

@cla-bot cla-bot Bot added the cla-signed label May 30, 2024
@ggilmore Graphite App

ggilmore commented May 30, 2024

Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

Join @ggilmore and the rest of your teammates on Graphite Graphite

@github-actions github-actions Bot added team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all labels May 30, 2024
@ggilmore ggilmore marked this pull request as ready for review May 30, 2024 00:03
ggilmore
ggilmore commented May 30, 2024
View reviewed changes
Comment thread cmd/frontend/graphqlbackend/external_service.go Outdated

@ggilmore ggilmore May 30, 2024

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

We had no timeout on connection checks before - so it could just spin endlessly. I added one here with a timeout that seemed reasonable.

@eseliger eseliger Jun 3, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch

@ggilmore ggilmore force-pushed the 05-29-wip_try_adding_note_to_repository_check_page_about_publically_accessible_urls branch from bde4254 to f40b6c7 Compare May 30, 2024 19:49
@ggilmore ggilmore force-pushed the 05-29-wip_try_adding_note_to_repository_check_page_about_publically_accessible_urls branch from f40b6c7 to 25ff8eb Compare May 30, 2024 19:50
@ggilmore ggilmore changed the title wip: try adding note to repository check page about publically accessible urls cmd: frontend: add note that clarifies that your code host needs to be accessible over the network to test connection button May 30, 2024
eseliger
eseliger approved these changes Jun 3, 2024
View reviewed changes
Comment thread cmd/frontend/graphqlbackend/external_service.go Outdated

@eseliger eseliger Jun 3, 2024

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Good catch

@graphite-app

graphite-app Bot commented Jun 3, 2024

Copy link
Copy Markdown

Celebrity gif. Idris Elba is sitting in the audience and he gives us a big thumbs up and a wink before turning to look back at the stage. (Added via Giphy)

@ggilmore ggilmore merged commit eb5d334 into main Jun 3, 2024
@ggilmore ggilmore deleted the 05-29-wip_try_adding_note_to_repository_check_page_about_publically_accessible_urls branch June 3, 2024 21:38
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

1 more reviewer

@eseliger eseliger eseliger approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

cla-signed team/product-platform team/source Tickets under the purview of Source - the one Source to graph it all

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@ggilmore @eseliger