Skip to content

Add specific message for network errors on app launch (Web UI) #25527

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

Merged
gabrielcorado merged 5 commits into from
May 3, 2023
Merged

Add specific message for network errors on app launch (Web UI) #25527

Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
2b50fe7
feat(web): add network error message for app launch
gabrielcorado May 3, 2023
e7d0ec2
Update web/packages/teleport/src/AppLauncher/AppLauncher.tsx
gabrielcorado May 3, 2023
8f1c696
refactor(web): update error message
gabrielcorado May 3, 2023
891d91d
Merge branch 'master' into gabrielcorado/app-ssl-error-message
gabrielcorado May 3, 2023
136e89c
chore(web): fix linter
gabrielcorado May 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 8 additions & 4 deletions web/packages/teleport/src/AppLauncher/AppLauncher.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,12 @@ export function AppLauncher() {
const queryParams = new URLSearchParams(search);

const createAppSession = useCallback(async (params: UrlLauncherParams) => {
let fqdn = params.fqdn;
const port = location.port ? `:${location.port}` : '';

try {
let fqdn = params.fqdn;
if (!fqdn) {
const app = await service.getAppFqdn(params);

fqdn = app.fqdn;
}

Expand All @@ -48,7 +49,6 @@ export function AppLauncher() {
params.arn = decodeURIComponent(params.arn);
}

const port = location.port ? `:${location.port}` : '';
const session = await service.createAppSession(params);

await fetch(`https://${fqdn}${port}/x-teleport-auth`, {
Expand All @@ -72,7 +72,11 @@ export function AppLauncher() {
window.location.replace(`https://${fqdn}${port}${path}`);
} catch (err) {
let statusText = 'Something went wrong';
if (err instanceof Error) {

if (err instanceof TypeError) {
Copy link
Copy Markdown
Collaborator

@r0mant r0mant May 3, 2023

Choose a reason for hiding this comment

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

I'm no expert but TypeError seems broad. Would be it better to check if the error text is "Failed to fetch" like what we see is being returned in this case?

@ryanclark @kimlisa I'll defer to you on this one.

Copy link
Copy Markdown
Contributor

@kimlisa kimlisa May 3, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

roman has a point b/c there could be other TypeError's like accessing a field in an undefined variable (seems unlikely though), however different browsers will have different error messages for network errors. tested them out:

chrome: failed to fetch
firefox: network error when attempting to fetch resource.
safari: load failed

so i think the way it is okay, as long as we are aware it could mean something else which the web console will print out the real error

Copy link
Copy Markdown
Contributor

@kimlisa kimlisa May 3, 2023

Choose a reason for hiding this comment

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

or we could just add the real error message err.message along with the status text

// `fetch` returns `TypeError` when there is a network error.
statusText = `Unable to access "${fqdn}". This may happen if your Teleport Proxy is using untrusted or self-signed certificate. Please ensure Teleport Proxy service uses valid certificate or access the application domain directly (https://${fqdn}${port}) and accept the certificate exception from your browser.`;
} else if (err instanceof Error) {
statusText = err.message;
}

Expand Down