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

Add a stacktrace-inducing template token #2442

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ribasushi
Copy link

Having err objects respond to %+v is quite widespread within the golang ecosystem. Add a logger template unit supporting this behavior.

Copy link
Contributor

@aldas aldas left a comment

Choose a reason for hiding this comment

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

This change is missing tests. Is it tested? Is the goal to use error structs that have custom String method or what the goal is? I do not understand without examples because %v when used for error will call Error() method.

See these examples - is there any difference in output?

This is using ${error_stacktrace} as template

type customError string

func (c customError) Error() string {
	return "err_" + string(c)
}
func (c customError) String() string {
	return "string_" + string(c)
}

func TestXXX(t *testing.T) {
	var testCases = []struct {
		name        string
		when        error
		whenNoError bool
		expect      string
	}{
		{
			name:   "plain errors",
			when:   errors.New("my error"),
			expect: `{"method":"GET","error":"my error"}`,
		},
		{
			name:   "echo errors",
			when:   echo.ErrBadGateway,
			expect: `{"method":"GET","error":"code=502, message=Bad Gateway"}`,
		},
		{
			name:   "custom stringer error",
			when:   customError("my stringer"),
			expect: `{"method":"GET","error":"err_my stringer"}`, // %+v does not call String()
		},
	}
	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			e := echo.New()
			buf := new(bytes.Buffer)
			e.Use(LoggerWithConfig(LoggerConfig{
				Format: `{"method":"${method}","error":"${error_stacktrace}"}` + "\n",
				Output: buf,
			}))

			e.GET("/", func(c echo.Context) error {
				if tc.whenNoError {
					return c.String(http.StatusOK, "OK")
				}
				return tc.when
			})

			req := httptest.NewRequest(http.MethodGet, "/", nil)
			rec := httptest.NewRecorder()
			e.ServeHTTP(rec, req)

			assert.Equal(t, tc.expect+"\n", buf.String())
		})
	}
}

This is using ${error} as template

type customError string

func (c customError) Error() string {
	return "err_" + string(c)
}
func (c customError) String() string {
	return "string_" + string(c)
}

func TestXXX2(t *testing.T) {
	var testCases = []struct {
		name        string
		when        error
		whenNoError bool
		expect      string
	}{
		{
			name:   "plain errors",
			when:   errors.New("my error"),
			expect: `{"method":"GET","error":"my error"}`,
		},
		{
			name:   "echo errors",
			when:   echo.ErrBadGateway,
			expect: `{"method":"GET","error":"code=502, message=Bad Gateway"}`,
		},
		{
			name:   "custom stringer error",
			when:   customError("my stringer"),
			expect: `{"method":"GET","error":"err_my stringer"}`, // %+v does not call String()
		},
	}
	for _, tc := range testCases {
		t.Run(tc.name, func(t *testing.T) {
			e := echo.New()
			buf := new(bytes.Buffer)
			e.Use(LoggerWithConfig(LoggerConfig{
				Format: `{"method":"${method}","error":"${error}"}` + "\n",
				Output: buf,
			}))

			e.GET("/", func(c echo.Context) error {
				if tc.whenNoError {
					return c.String(http.StatusOK, "OK")
				}
				return tc.when
			})

			req := httptest.NewRequest(http.MethodGet, "/", nil)
			rec := httptest.NewRecorder()
			e.ServeHTTP(rec, req)

			assert.Equal(t, tc.expect+"\n", buf.String())
		})
	}
}

@zubairalam
Copy link

Hi @aldas . Can I pick this if it needs to cleaned?

@ribasushi
Copy link
Author

@aldas apologies for the radio silence. Was not tested originally as it wasn't clear what would substitute a reasonable test. Please observe this playground for the real-world-use motivation behind this change.

Would you accept a test using stacked xerrors.Errorf(... %w) ?

Having `err` objects respond to `%+v` is quite widespread
within the golang ecosystem. Add a logger template unit
supporting this behavior.
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.

None yet

3 participants