gin-contrib / timeout

Timeout middleware for Gin
MIT License
185 stars 38 forks source link

HTTP Code 204 is not properly handled #31

Closed ic2hrmk closed 1 year ago

ic2hrmk commented 2 years ago

Hello,

I reached to the point when I'm responding within the handler with the status code 204 (No content), but the timeout middleware changes the code to 200 (OK).

To reproduce, run the following test:

package middleware_test

import (
    "log"
    "net/http"
    "net/http/httptest"
    "net/http/httputil"
    "testing"
    "time"

    "github.com/gin-contrib/timeout"
    "github.com/gin-gonic/gin"
)

func Timeout(duration time.Duration) gin.HandlerFunc {
    return timeout.New(
        timeout.WithTimeout(duration),
        timeout.WithHandler(func(c *gin.Context) { c.Next() }),
        timeout.WithResponse(timeoutHandler()),
    )
}

func timeoutHandler() gin.HandlerFunc {
    gatewayTimeoutErr := struct {
        Error string `json:"error"`
    }{
        Error: "Timed out.",
    }

    return func(c *gin.Context) {
        log.Printf("request timed out: [method=%s,path=%s]",
            c.Request.Method, c.Request.URL.Path)
        c.JSON(http.StatusGatewayTimeout, gatewayTimeoutErr)
    }
}

func Test2XXCode(t *testing.T) {
    gin.SetMode(gin.ReleaseMode)

    type testCase struct {
        Name          string
        Method        string
        Path          string
        ExpStatusCode int
        Handler       gin.HandlerFunc
    }

    var (
        testTimeOutDuration = 1 * time.Second

        cases = []testCase{
            {
                Name:          "Plain text (200)",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusOK,
                Handler: func(ctx *gin.Context) {
                    ctx.String(http.StatusOK, "I'm text!")
                },
            },
            {
                Name:          "Plain text (201)",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusCreated,
                Handler: func(ctx *gin.Context) {
                    ctx.String(http.StatusCreated, "I'm created!")
                },
            },
            {
                Name:          "Plain text (204)",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusNoContent,
                Handler: func(ctx *gin.Context) {
                    ctx.String(http.StatusNoContent, "")
                },
            },
            {
                Name:          "JSON (200)",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusOK,
                Handler: func(ctx *gin.Context) {
                    ctx.JSON(http.StatusOK, gin.H{"field": "value"})
                },
            },
            {
                Name:          "JSON (201)",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusCreated,
                Handler: func(ctx *gin.Context) {
                    ctx.JSON(http.StatusCreated, gin.H{"field": "value"})
                },
            },
            {
                Name:          "JSON (204)",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusNoContent,
                Handler: func(ctx *gin.Context) {
                    ctx.JSON(http.StatusNoContent, nil)
                },
            },
            {
                Name:          "No reply",
                Method:        http.MethodGet,
                Path:          "/me",
                ExpStatusCode: http.StatusOK,
                Handler:       func(ctx *gin.Context) {},
            },
        }

        initCase = func(c testCase) (*http.Request, *httptest.ResponseRecorder) {
            return httptest.NewRequest(c.Method, c.Path, nil), httptest.NewRecorder()
        }
    )

    for i := range cases {
        t.Run(cases[i].Name, func(tt *testing.T) {
            tt.Logf("Test case [%s]", cases[i].Name)

            router := gin.Default()

            router.Use(Timeout(testTimeOutDuration))
            router.GET("/*root", cases[i].Handler)

            req, resp := initCase(cases[i])
            router.ServeHTTP(resp, req)

            rawResponse, _ := httputil.DumpResponse(resp.Result(), true)
            log.Printf("===============================================\n%s\n"+
                "===============================================", string(rawResponse))

            if resp.Code != cases[i].ExpStatusCode {
                tt.Errorf("response is different from expected:\nexp: >>>%d<<<\ngot: >>>%d<<<",
                    cases[i].ExpStatusCode, resp.Code)
            }
        })
    }
}

Results I retrieved:

===============================================
--- FAIL: Test2XXCode (0.00s)
    --- PASS: Test2XXCode/Plain_text_(200) (0.00s)
    --- PASS: Test2XXCode/Plain_text_(201) (0.00s)
    --- FAIL: Test2XXCode/Plain_text_(204) (0.00s)

    --- PASS: Test2XXCode/JSON_(200) (0.00s)
    --- PASS: Test2XXCode/JSON_(201) (0.00s)
    --- FAIL: Test2XXCode/JSON_(204) (0.00s)

    --- PASS: Test2XXCode/No_reply (0.00s)
FAIL

To confirm the issue, exclude the line router.Use(Timeout(testTimeOutDuration)) from the test and run it again, it would pass then.

Edit: github.com/gin-contrib/timeout is the latest, from the master branch (commit: 53932ea)

jeff-lyu commented 1 year ago

add ‘w.ResponseWriter.WriteHeader(code)’ within 'func (w *TimeoutWriter) WriteHeader(code int)'

appleboy commented 1 year ago

fixed in https://github.com/gin-contrib/timeout/releases/tag/v0.0.5