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

Render aggregated errors properly #104

Open
hugoShaka opened this issue Dec 11, 2023 · 1 comment
Open

Render aggregated errors properly #104

hugoShaka opened this issue Dec 11, 2023 · 1 comment

Comments

@hugoShaka
Copy link

hugoShaka commented Dec 11, 2023

Wrapping and aggregating errors cause the error report to be unreadable. This is the number 1 cause of confusion for users attempting to join an instance to a Teleport cluster. Users cannot read the join error properly because it's malformed: they focus on the last part of the error "certificate is not valid" (coming from the direct auth join attempt) and ignore the actual cause (something broken during proxy joining).

Example

The following go code:

package main

import (
	"fmt"
	"github.com/gravitational/trace"
)

func main() {
	proxyErr := trace.Wrap(trace.BadParameter("the fizzbuzz buzzed too hard"), "joining via proxy")
	authErr := trace.Wrap(trace.BadParameter("auth server unavailable: cert invalid"), "joining via auth server")
	err := trace.Wrap(trace.NewAggregate(proxyErr, authErr), "failed to join")
	fmt.Println(trace.UserMessage(err))
}

outputs:

failed to join
        joining via proxy
        the fizzbuzz buzzed too hard, joining via auth server
        auth server unavailable: cert invalid

A sane output would be:

failed to join
        joining via proxy
                the fizzbuzz buzzed too hard
        joining via auth server
                auth server unavailable: cert invalid

Implementation suggestion

We could kill two birds in one stone by changing the trace.Wrap behaviour to encapsulate already wrapped errors instead of adding a message toError.Messages. This would make trace behave like go's standard wrapping and allow us to easily build an error tree. With an error tree, fixing the error rendering would be way easier.

This would likely mean doing a trace/v2 library but would allow us to better integrate with the go ecosystem and address other outstanding trace issues.

Related errors

@strideynet
Copy link
Contributor

One thing worth considering is that the Go standard library now provides support for multiple wrapped errors - as much as we try to workaround in trace, it doesn't deal with the fact that some library we use may make use of the ability to wrap multiple aggregate errors. This suggests to me that we ought to take some steps to align trace better with the practices and APIs that have emerged in Go so that we aren't "fighting the tide".

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

No branches or pull requests

2 participants