-
Notifications
You must be signed in to change notification settings - Fork 295
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
Handle network disconnection #107
Changes from all commits
cd990c9
055cb8c
6811f02
2885645
2b9d6a8
19041b3
2f67c89
3f6c412
3016e05
12100c8
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,5 +1,9 @@ | ||
import { VSCodeButton } from '@vscode/webview-ui-toolkit/react' | ||
|
||
import { AuthStatus, isLocalApp } from '../src/chat/protocol' | ||
|
||
import { getVSCodeAPI } from './utils/VSCodeApi' | ||
|
||
import styles from './Login.module.css' | ||
|
||
const AUTH_ERRORS = { | ||
|
@@ -10,6 +14,7 @@ const AUTH_ERRORS = { | |
EMAIL_NOT_VERIFIED: 'Email not verified. Please add a verified email to your Sourcegraph.com account.', | ||
APP_NOT_RUNNING: 'Cody App is not running. Please open the Cody App to continue.', | ||
INVALID_URL: 'Connection failed due to invalid URL. Please enter a valid Sourcegraph instance URL.', | ||
NETWORK_ERROR: 'Connection failed due to a network problem. Please check your internet connection and try again.', | ||
} | ||
export const ErrorContainer: React.FunctionComponent<{ | ||
authStatus: AuthStatus | ||
|
@@ -26,11 +31,12 @@ export const ErrorContainer: React.FunctionComponent<{ | |
requiresVerifiedEmail, | ||
hasVerifiedEmail, | ||
siteVersion, | ||
showNetworkError, | ||
} = authStatus | ||
// Version is compatible if this is an insider build or version is 5.1.0 or above | ||
// Right now we assumes all insider builds have Cody enabled | ||
// NOTE: Insider build includes App builds but we should seperate them in the future | ||
if (!authenticated && !showInvalidAccessTokenError) { | ||
if (!authenticated && !showInvalidAccessTokenError && !showNetworkError) { | ||
return null | ||
} | ||
// Errors for app are handled in the ConnectApp component | ||
|
@@ -45,6 +51,26 @@ export const ErrorContainer: React.FunctionComponent<{ | |
const isVersionCompatible = isInsiderBuild || siteVersion >= '5.1.0' | ||
const isVersionBeforeCody = !isVersionCompatible && siteVersion < '5.0.0' | ||
const prefix = `Failed: ${isApp.isRunning ? 'Cody App' : endpoint}` | ||
|
||
const handleReload = (): void => { | ||
getVSCodeAPI().postMessage({ command: 'reload' }) | ||
} | ||
|
||
// When there's a network error | ||
if (showNetworkError) { | ||
return ( | ||
<> | ||
<div className={styles.error}> | ||
<p>Current endpoint: {endpoint}</p> | ||
<p>{AUTH_ERRORS.NETWORK_ERROR}</p> | ||
<VSCodeButton className={styles.button} type="button" onClick={handleReload}> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. maybe update the button styles to have full width like the rest of the buttons in the login page? Will defer to @toolmantim who will be back next week from his PTO :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, we can have that so it would be align to the style with all other buttons. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd love us to move away from the-full width buttons… but it's good to be consistent. But given it's an error state, I'm okay with just leaving this as a normal button w/ inherent width. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks @toolmantim |
||
Reload | ||
</VSCodeButton> | ||
</div> | ||
</> | ||
) | ||
} | ||
|
||
// When doesn't have a valid token | ||
if (showInvalidAccessTokenError) { | ||
return ( | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like this! Maybe we can show the endpoint address to let the user know which instance is having network issue?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That would be a good add on, I will update it.