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

Troubleshoot: Show auth connection issues #3750

Merged
merged 27 commits into from
Apr 17, 2024

Conversation

RXminuS
Copy link
Contributor

@RXminuS RXminuS commented Apr 9, 2024

Fixes #3673 where we mistakingly show a signIn screen if there are connectivity issues to the Cody server.

CleanShot 2024-04-17 at 14 46 44@2x

Test plan

  • End2end tests are added
  • Storybook is added to view the UI
    • Allow for full-viewport rendering to test for breakpoint issues.

Extends the UI to cover the full viewport making it easier to try the UI on different sizes.
@RXminuS RXminuS self-assigned this Apr 9, 2024
@RXminuS RXminuS requested a review from arafatkatze April 9, 2024 14:29
@RXminuS RXminuS changed the title VS Code: Troubleshoot auth connection issues Troubleshoot: Surface auth connection issues Apr 11, 2024
@RXminuS RXminuS force-pushed the rnauta/troubleshooting/connection_issues branch from 6a7f279 to 6de5e4c Compare April 11, 2024 11:59
@RXminuS RXminuS marked this pull request as ready for review April 11, 2024 11:59
@RXminuS
Copy link
Contributor Author

RXminuS commented Apr 11, 2024

@sourcegraph/cody-clients Does this affect JB's in any way? I had to update the Kotlin protocols but I'm not sure if that was the correct approach.

Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

Left some comments inline. The only concern i have is about the reset function, but the rest looks good to me. Nice work!

vscode/src/chat/protocol.ts Outdated Show resolved Hide resolved
vscode/src/chat/protocol.ts Show resolved Hide resolved
vscode/src/chat/chat-view/SidebarViewController.ts Outdated Show resolved Hide resolved
vscode/src/services/utils/troubleshoot.ts Outdated Show resolved Hide resolved
vscode/src/services/utils/troubleshoot.ts Outdated Show resolved Hide resolved
vscode/src/services/utils/troubleshoot.ts Outdated Show resolved Hide resolved
vscode/webviews/App.tsx Outdated Show resolved Hide resolved
@RXminuS
Copy link
Contributor Author

RXminuS commented Apr 12, 2024

Yeah I'll probably just change that to a sign out. I just didn't want someone to get stuck with bad settings and not get back to the sign in screen.

Thanks for taking a look

@RXminuS
Copy link
Contributor Author

RXminuS commented Apr 13, 2024

I've decided to remove the "fullReset" and "showOutput" logs in favor of another feature coming soon to help users enable and navigate debug mode/troubleshooting mode more easily. So this screen will only allow you to retry or sign-out explicitly. The problem with some of the other commands is that they are tied to specific UI clicks and handle all the analytics as well (which I think is a different problem altogether). But I'll see if I can simplify it somewhat.

Although there will be 4-5 commands coming to the troubleshoot namespace soon

- Only offer sign-out to prevent the user accidentally ending up in a locked state if anything is wrong with the auth. (e.g. switched endpoints)
- The debug / full reset functionality will be moved to specific UI & features in coming PR's
@RXminuS RXminuS removed the request for review from arafatkatze April 16, 2024 12:00
Copy link

‼️ Hey @sourcegraph/cody-security, please review this PR carefully as it introduces usage of unsafe_ functions or abuses PromptString.

@philipp-spiess
Copy link
Contributor

philipp-spiess commented Apr 16, 2024

@RXminuS sorry regarding the message above, see: #3797 (comment)

Fix incoming: #3810

@RXminuS RXminuS requested a review from abeatrix April 16, 2024 17:34
@RXminuS
Copy link
Contributor Author

RXminuS commented Apr 16, 2024

@philipp-spiess increasing the timeout fixed the e2e tests 👍 thanks for the help

@RXminuS RXminuS changed the title Troubleshoot: Surface auth connection issues Troubleshoot: Show auth connection issues Apr 16, 2024
@toolmantim
Copy link
Contributor

Nice! This design is looking good too.

For the width/positioning, use VS Code’s Run & Debug panel and responsive breakpoints as the guide:

CleanShot.2024-04-17.at.11.24.01.mp4

i.e. icon & buttons are full width until a point, then left aligned

@RXminuS RXminuS removed the request for review from philipp-spiess April 17, 2024 01:33
@RXminuS
Copy link
Contributor Author

RXminuS commented Apr 17, 2024

Nice! This design is looking good too.

For the width/positioning, use VS Code’s Run & Debug panel and responsive breakpoints as the guide:

@toolmantim VScode had a few inconsistent breakpoints so I just picked 650px as the max. They also have global flags for reduced motion that I didn't want to dive into now so I didn't add any animations. I think that's something we can globally define once we set up Tailwind helpers. But for now, I've tried to follow the spirit of it :-)

CleanShot.2024-04-17.at.14.36.07.mp4

@RXminuS RXminuS enabled auto-merge (squash) April 17, 2024 13:15
vscode/src/main.ts Outdated Show resolved Hide resolved
This reverts commit 56e1ab7.

Revert "Status bar loading state"

This reverts commit 8ced982.
Copy link
Contributor

@abeatrix abeatrix left a comment

Choose a reason for hiding this comment

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

Thanks for working on this and for adding all those amazing tests! Confirm the chat panel is now showing up correctly on reload, and we can look into the Loading page issue separately 😄 well done!

@RXminuS RXminuS merged commit 282aa57 into main Apr 17, 2024
21 checks passed
@RXminuS RXminuS deleted the rnauta/troubleshooting/connection_issues branch April 17, 2024 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

bug: Connection issues can lead to seemingly signing-out
4 participants