From 0709ae3b103b3572fa9154e9028569c5dfa772fd Mon Sep 17 00:00:00 2001 From: eeonevision Date: Mon, 25 Nov 2019 11:33:12 +0300 Subject: [PATCH 01/10] Removed useless panicking from renderer/context --- context.go | 7 ++++++- render/json.go | 14 ++------------ 2 files changed, 8 insertions(+), 13 deletions(-) diff --git a/context.go b/context.go index 046f284ece..4bbd41d3d9 100644 --- a/context.go +++ b/context.go @@ -16,6 +16,7 @@ import ( "net/url" "os" "strings" + "syscall" "time" "github.com/gin-contrib/sse" @@ -814,7 +815,11 @@ func (c *Context) Render(code int, r render.Render) { } if err := r.Render(c.Writer); err != nil { - panic(err) + c.Error(err) + + if err == syscall.EPIPE { + c.AbortWithStatus(httpStatusClientClosedRequest) + } } } diff --git a/render/json.go b/render/json.go index 70506f78a8..75dad12356 100644 --- a/render/json.go +++ b/render/json.go @@ -54,10 +54,8 @@ var jsonAsciiContentType = []string{"application/json"} // Render (JSON) writes data with custom ContentType. func (r JSON) Render(w http.ResponseWriter) (err error) { - if err = WriteJSON(w, r.Data); err != nil { - panic(err) - } - return + r.WriteContentType(w) + return json.NewEncoder(w).Encode(r.Data) } // WriteContentType (JSON) writes JSON ContentType. @@ -65,14 +63,6 @@ func (r JSON) WriteContentType(w http.ResponseWriter) { writeContentType(w, jsonContentType) } -// WriteJSON marshals the given interface object and writes it with custom ContentType. -func WriteJSON(w http.ResponseWriter, obj interface{}) error { - writeContentType(w, jsonContentType) - encoder := json.NewEncoder(w) - err := encoder.Encode(&obj) - return err -} - // Render (IndentedJSON) marshals the given interface object and writes it with custom ContentType. func (r IndentedJSON) Render(w http.ResponseWriter) error { r.WriteContentType(w) From e0ca0b16ab99c7a44dcbfade85bc9d588d6dceb7 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Mon, 25 Nov 2019 11:35:11 +0300 Subject: [PATCH 02/10] Added custom http status code to context's render --- context.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/context.go b/context.go index 4bbd41d3d9..bbf1fc034d 100644 --- a/context.go +++ b/context.go @@ -39,6 +39,8 @@ const ( const abortIndex int8 = math.MaxInt8 / 2 +const httpStatusClientClosedRequest = 499 + // Context is the most important part of gin. It allows us to pass variables between middleware, // manage the flow, validate the JSON of a request and render a JSON response for example. type Context struct { From edbd1abf2e8e62cda4aafaa181ae1d52d8ebd468 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Mon, 25 Nov 2019 11:52:19 +0300 Subject: [PATCH 03/10] Updated context's unit tests --- context_test.go | 12 +++++------- 1 file changed, 5 insertions(+), 7 deletions(-) diff --git a/context_test.go b/context_test.go index 18709d3dcd..9d2809cd01 100644 --- a/context_test.go +++ b/context_test.go @@ -31,6 +31,8 @@ import ( var _ context.Context = &Context{} +var errTestPanicRender = errors.New("TestPanicRender") + // Unit tests TODO // func (c *Context) File(filepath string) { // func (c *Context) Negotiate(code int, config Negotiate) { @@ -634,22 +636,18 @@ type TestPanicRender struct { } func (*TestPanicRender) Render(http.ResponseWriter) error { - return errors.New("TestPanicRender") + return errTestPanicRender } func (*TestPanicRender) WriteContentType(http.ResponseWriter) {} -func TestContextRenderPanicIfErr(t *testing.T) { - defer func() { - r := recover() - assert.Equal(t, "TestPanicRender", fmt.Sprint(r)) - }() +func TestContextRenderIfErr(t *testing.T) { w := httptest.NewRecorder() c, _ := CreateTestContext(w) c.Render(http.StatusOK, &TestPanicRender{}) - assert.Fail(t, "Panic not detected") + assert.Equal(t, errorMsgs{&Error{Err: errTestPanicRender, Type: 1}}, c.Errors) } // Tests that the response is serialized as JSON From f5c2b09526102dab3600e1040755b6fee833b785 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Mon, 25 Nov 2019 12:12:12 +0300 Subject: [PATCH 04/10] Fixed render test --- render/render_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/render/render_test.go b/render/render_test.go index 376733df5f..7f413813c6 100644 --- a/render/render_test.go +++ b/render/render_test.go @@ -66,12 +66,12 @@ func TestRenderJSON(t *testing.T) { assert.Equal(t, "application/json; charset=utf-8", w.Header().Get("Content-Type")) } -func TestRenderJSONPanics(t *testing.T) { +func TestRenderJSONError(t *testing.T) { w := httptest.NewRecorder() data := make(chan int) // json: unsupported type: chan int - assert.Panics(t, func() { assert.NoError(t, (JSON{data}).Render(w)) }) + assert.Error(t, (JSON{data}).Render(w)) } func TestRenderIndentedJSON(t *testing.T) { From ea0b535a8d363cbafaf149a28dcd805516b7cc25 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Tue, 26 Nov 2019 16:21:04 +0300 Subject: [PATCH 05/10] Removed broken pipe handling condition --- context.go | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/context.go b/context.go index bbf1fc034d..3afef3121b 100644 --- a/context.go +++ b/context.go @@ -16,7 +16,6 @@ import ( "net/url" "os" "strings" - "syscall" "time" "github.com/gin-contrib/sse" @@ -818,10 +817,7 @@ func (c *Context) Render(code int, r render.Render) { if err := r.Render(c.Writer); err != nil { c.Error(err) - - if err == syscall.EPIPE { - c.AbortWithStatus(httpStatusClientClosedRequest) - } + c.Abort() } } From 53813826951152f1433d72f3d2e972c7e2c55aa5 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Wed, 27 Nov 2019 18:28:13 +0300 Subject: [PATCH 06/10] Returned public WriteJSON method --- render/json.go | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/render/json.go b/render/json.go index 75dad12356..4e90f783bf 100644 --- a/render/json.go +++ b/render/json.go @@ -63,6 +63,14 @@ func (r JSON) WriteContentType(w http.ResponseWriter) { writeContentType(w, jsonContentType) } +// WriteJSON marshals the given interface object and writes it with custom ContentType. +func WriteJSON(w http.ResponseWriter, obj interface{}) error { + writeContentType(w, jsonContentType) + encoder := json.NewEncoder(w) + err := encoder.Encode(&obj) + return err +} + // Render (IndentedJSON) marshals the given interface object and writes it with custom ContentType. func (r IndentedJSON) Render(w http.ResponseWriter) error { r.WriteContentType(w) From f078613fb22ec76391acb58f2509410c67c585e7 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Fri, 29 Nov 2019 19:25:46 +0300 Subject: [PATCH 07/10] Removed orphan http constant --- context.go | 2 -- 1 file changed, 2 deletions(-) diff --git a/context.go b/context.go index 3afef3121b..d14ff6c970 100644 --- a/context.go +++ b/context.go @@ -38,8 +38,6 @@ const ( const abortIndex int8 = math.MaxInt8 / 2 -const httpStatusClientClosedRequest = 499 - // Context is the most important part of gin. It allows us to pass variables between middleware, // manage the flow, validate the JSON of a request and render a JSON response for example. type Context struct { From d0b7ad21cedc75cdef1630f68a8ca911e648d8a6 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Tue, 10 Jan 2023 15:50:02 +0300 Subject: [PATCH 08/10] Fixed json renderer --- render/json.go | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/render/json.go b/render/json.go index e562fd712c..55c441fab9 100644 --- a/render/json.go +++ b/render/json.go @@ -54,8 +54,7 @@ var ( // Render (JSON) writes data with custom ContentType. func (r JSON) Render(w http.ResponseWriter) (err error) { - r.WriteContentType(w) - return json.NewEncoder(w).Encode(r.Data) + return WriteJSON(w, r.Data) } // WriteContentType (JSON) writes JSON ContentType. From 9a45526592626444be4a9aced8bd28128d6a1390 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Tue, 10 Jan 2023 16:00:46 +0300 Subject: [PATCH 09/10] Fixed error handling in context render --- context.go | 3 ++- render/json.go | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/context.go b/context.go index 5a28df7983..2b7d515913 100644 --- a/context.go +++ b/context.go @@ -924,7 +924,8 @@ func (c *Context) Render(code int, r render.Render) { } if err := r.Render(c.Writer); err != nil { - c.Error(err) + // Pushing error to c.Errors + _ = c.Error(err) c.Abort() } } diff --git a/render/json.go b/render/json.go index 55c441fab9..fc8dea453f 100644 --- a/render/json.go +++ b/render/json.go @@ -53,7 +53,7 @@ var ( ) // Render (JSON) writes data with custom ContentType. -func (r JSON) Render(w http.ResponseWriter) (err error) { +func (r JSON) Render(w http.ResponseWriter) error { return WriteJSON(w, r.Data) } From f32da2dc73b5c6d63ee17ffbd142616cf2f99fe5 Mon Sep 17 00:00:00 2001 From: eeonevision Date: Tue, 10 Jan 2023 16:09:18 +0300 Subject: [PATCH 10/10] Renamed panic render to render in context test --- context_test.go | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/context_test.go b/context_test.go index f3ee1095c0..2ebdc3087c 100644 --- a/context_test.go +++ b/context_test.go @@ -32,7 +32,7 @@ import ( var _ context.Context = (*Context)(nil) -var errTestPanicRender = errors.New("TestPanicRender") +var errTestRender = errors.New("TestRender") // Unit tests TODO // func (c *Context) File(filepath string) { @@ -645,21 +645,21 @@ func TestContextBodyAllowedForStatus(t *testing.T) { assert.True(t, true, bodyAllowedForStatus(http.StatusInternalServerError)) } -type TestPanicRender struct{} +type TestRender struct{} -func (*TestPanicRender) Render(http.ResponseWriter) error { - return errTestPanicRender +func (*TestRender) Render(http.ResponseWriter) error { + return errTestRender } -func (*TestPanicRender) WriteContentType(http.ResponseWriter) {} +func (*TestRender) WriteContentType(http.ResponseWriter) {} func TestContextRenderIfErr(t *testing.T) { w := httptest.NewRecorder() c, _ := CreateTestContext(w) - c.Render(http.StatusOK, &TestPanicRender{}) + c.Render(http.StatusOK, &TestRender{}) - assert.Equal(t, errorMsgs{&Error{Err: errTestPanicRender, Type: 1}}, c.Errors) + assert.Equal(t, errorMsgs{&Error{Err: errTestRender, Type: 1}}, c.Errors) } // Tests that the response is serialized as JSON