samber / slog-fiber

🚨 Fiber middleware for slog logger
https://pkg.go.dev/github.com/samber/slog-fiber
MIT License
53 stars 9 forks source link

fix: Incorrect response.status code #19

Closed dassump closed 5 months ago

dassump commented 6 months ago

For the status code to be correctly captured by the c.Response().StatusCode(), the response must be dispatched to the client first. To do this, we need to call the ErrorHandler right after c.Next() in the middleware. By analyzing standard fiber logging middleware, we can see this.

Using the same code of #18 for testing

Test 1: Get / (Expected: 200 | Received: 200)

$ curl -X GET localhost:8080/

time=2024-03-08T00:36:26.816-03:00 level=INFO msg="Incoming request" request.time=2024-03-08T00:36:26.816-03:00 request.method=GET request.host=localhost:8080 request.path=/ request.query="" request.params=map[] request.route=/ request.ip=127.0.0.1 request.x-forwarded-for=[] request.referer="" request.length=0 response.time=2024-03-08T00:36:26.816-03:00 response.latency=46.792µs response.status=200 response.length=2 id=37661376-14ce-4508-9b45-63b7cb18ce6a

Test 2: Post / (Expected: 405 | Received: 405)

$ curl -X POST localhost:8080/

time=2024-03-08T00:36:26.828-03:00 level=WARN msg="Method Not Allowed" request.time=2024-03-08T00:36:26.828-03:00 request.method=POST request.host=localhost:8080 request.path=/ request.query="" request.params=map[] request.route=/ request.ip=127.0.0.1 request.x-forwarded-for=[] request.referer="" request.length=0 response.time=2024-03-08T00:36:26.828-03:00 response.latency=45.917µs response.status=405 response.length=18 id=40d5c6c9-6497-44da-8636-c1bc62295718

Test 3: Post /bad (Expected: 400 | Received: 400)

$ curl -X POST localhost:8080/bad

time=2024-03-08T00:36:26.838-03:00 level=WARN msg="Bad Request" request.time=2024-03-08T00:36:26.838-03:00 request.method=POST request.host=localhost:8080 request.path=/bad request.query="" request.params=map[] request.route=/bad request.ip=127.0.0.1 request.x-forwarded-for=[] request.referer="" request.length=0 response.time=2024-03-08T00:36:26.838-03:00 response.latency=12.667µs response.status=400 response.length=11 id=32ff227d-4fa3-4abc-b3f6-ca25e861059c

Test 4: Get /die (Expected: 500 | Received: 500)

$ curl -X GET localhost:8080/die

time=2024-03-08T00:36:26.848-03:00 level=ERROR msg=OK request.time=2024-03-08T00:36:26.848-03:00 request.method=GET request.host=localhost:8080 request.path=/die request.query="" request.params=map[] request.route=/die request.ip=127.0.0.1 request.x-forwarded-for=[] request.referer="" request.length=0 response.time=2024-03-08T00:36:26.848-03:00 response.latency=28.708µs response.status=500 response.length=2 id=7c10ed18-22cc-4b75-91c7-cb4b6d243be9

Test 5: Post /force (Expected: 401 | Received: 401)

$ curl -X POST localhost:8080/force

time=2024-03-08T00:36:26.858-03:00 level=WARN msg=Unauthorized request.time=2024-03-08T00:36:26.858-03:00 request.method=POST request.host=localhost:8080 request.path=/force request.query="" request.params=map[] request.route=/force request.ip=127.0.0.1 request.x-forwarded-for=[] request.referer="" request.length=0 response.time=2024-03-08T00:36:26.858-03:00 response.latency=10µs response.status=401 response.length=12 id=377cf057-ebdf-46c4-aa3a-e8180a1e9e2b

Test 6: Get /notfound (Expected: 404 | Received: 404)

curl -X GET localhost:8080/notfound

time=2024-03-08T00:36:26.868-03:00 level=WARN msg="Cannot GET /notfound" request.time=2024-03-08T00:36:26.867-03:00 request.method=GET request.host=localhost:8080 request.path=/notfound request.query="" request.params=map[] request.route=/ request.ip=127.0.0.1 request.x-forwarded-for=[] request.referer="" request.length=0 response.time=2024-03-08T00:36:26.868-03:00 response.latency=19.542µs response.status=404 response.length=20 id=a2e55526-d6e8-4275-bf27-91bc1c2d69e0

For this control to happen through the ErrorHandler, if desired, the catch-all function must not exist.

qindj commented 5 months ago

same issue, can this be merged? thanks @samber

samber commented 5 months ago

Hi @qindj and @dassump

Thanks for your contrib. I was suggesting another method in #14, but using sync.Once seems much better.

Let's merge.