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

feat(client/electron): propagate structured errors from Go to TypeScript #2033

Merged
merged 21 commits into from
Jun 26, 2024

Conversation

jyyi1
Copy link
Contributor

@jyyi1 jyyi1 commented Jun 10, 2024

Added detailed error propagation from Golang to TypeScript for the desktop Outline Client (Windows & Linux).

New Error Details

Empty hostname
image

Port out-of-range
image

Current Version (v1.13.1)

Empty hostname
image

Port out-of-range
image

@github-actions github-actions bot added size/XXL and removed size/XL labels Jun 17, 2024
@jyyi1 jyyi1 marked this pull request as ready for review June 17, 2024 21:22
@jyyi1 jyyi1 requested a review from a team as a code owner June 17, 2024 21:22
@jyyi1 jyyi1 requested review from fortuna and sbruens June 17, 2024 21:22
@@ -47,6 +47,7 @@ const DNS_RESOLVERS = ['1.1.1.1', '9.9.9.9'];
// about the others.
export class GoVpnTunnel implements VpnTunnel {
private readonly tun2socks: GoTun2socks;
private readonly connectivityChecker: GoTun2socks;
Copy link
Collaborator

Choose a reason for hiding this comment

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

Why is this type GoTun2socks? Why do we need two?

Copy link
Contributor Author

@jyyi1 jyyi1 Jun 18, 2024

Choose a reason for hiding this comment

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

Because we are invoking the same binary with different arguments. And these two processes might run at the same time. For example, we will check connectivity here, and tun2socks is still running until we stop it later.

Copy link
Contributor

@sbruens sbruens left a comment

Choose a reason for hiding this comment

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

This looks great.

client/electron/go_vpn_tunnel.ts Outdated Show resolved Hide resolved
client/electron/go_vpn_tunnel.ts Show resolved Hide resolved
client/electron/index.ts Outdated Show resolved Hide resolved
client/electron/process.ts Outdated Show resolved Hide resolved
client/electron/process.ts Outdated Show resolved Hide resolved
client/electron/process.ts Outdated Show resolved Hide resolved
client/src/www/model/platform_error.ts Outdated Show resolved Hide resolved
client/src/www/model/platform_error.ts Outdated Show resolved Hide resolved
client/src/www/model/platform_error.ts Outdated Show resolved Hide resolved
client/src/www/model/platform_error.ts Show resolved Hide resolved
@jyyi1 jyyi1 requested review from sbruens and fortuna June 21, 2024 22:08
}
return nil
}

// newIllegalConfigErrorWithDetails creates a TypeScript parsable SSIllegalConfig error with detailed information.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be IllegalConfig?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants