π₯ feat: Support for SendEarlyHints
Description
Please provide a clear and concise description of the changes you've made and the problem they address. Include the purpose of the change, any relevant issues it solves, and the benefits it brings to the project. If this change introduces new features or adjustments, highlight them here.
Fixes #3211
Changes introduced
List the new features or adjustments introduced in this pull request. Provide details on benchmarks, documentation updates, changelog entries, and if applicable, the migration guide.
- [ ] Benchmarks: Describe any performance benchmarks and improvements related to the changes.
- [ ] Documentation Update: Detail the updates made to the documentation and links to the changed files.
- [ ] Changelog/What's New: Include a summary of the additions for the upcoming release notes.
- [ ] Migration Guide: If necessary, provide a guide or steps for users to migrate their existing code to accommodate these changes.
- [ ] API Alignment with Express: Explain how the changes align with the Express API.
- [ ] API Longevity: Discuss the steps taken to ensure that the new or updated APIs are consistent and not prone to breaking changes.
- [ ] Examples: Provide examples demonstrating the new features or changes in action.
Type of change
Please delete options that are not relevant.
- [ ] New feature (non-breaking change which adds functionality)
- [ ] Enhancement (improvement to existing features and functionality)
- [ ] Documentation update (changes to documentation)
- [ ] Performance improvement (non-breaking change which improves efficiency)
- [ ] Code consistency (non-breaking change which improves code reliability and robustness)
Checklist
Before you submit your pull request, please make sure you meet these requirements:
- [ ] Followed the inspiration of the Express.js framework for new functionalities, making them similar in usage.
- [ ] Conducted a self-review of the code and provided comments for complex or critical parts.
- [ ] Updated the documentation in the
/docs/directory for Fiber's documentation. - [ ] Added or updated unit tests to validate the effectiveness of the changes or new features.
- [ ] Ensured that new and existing unit tests pass locally with the changes.
- [ ] Verified that any new dependencies are essential and have been agreed upon by the maintainers/community.
- [ ] Aimed for optimal performance with minimal allocations in the new code.
- [ ] Provided benchmarks for the new code to analyze and improve upon.
Commit formatting
Please use emojis in commit messages for an easy way to identify the purpose or intention of a commit. Check out the emoji cheatsheet here: CONTRIBUTING.md
[!NOTE]
Other AI code review bot(s) detected
CodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review.
Walkthrough
Adds HTTP 103 Early Hints support: new Ctx.SendEarlyHints([]string) and Res.SendEarlyHints([]string) APIs and tests; updates SendFile signature to SendFile(file string, config ...SendFile); App.Test changed to parse/discard interim 1xx responses and use a testable httpReadResponse wrapper; tests added for early hints and App.Test edge cases.
Changes
| Cohort / File(s) | Summary |
|---|---|
Ctx API changesctx_interface_gen.go |
Adds SendEarlyHints(hints []string) error; changes SendFile(file string) error β SendFile(file string, config ...SendFile) error. |
Res API & implres_interface_gen.go, res.go |
Adds SendEarlyHints(hints []string) error to Res and implements func (r *DefaultRes) SendEarlyHints(hints []string) error (appends Link headers and calls fasthttp EarlyHints()). |
App test clientapp.go |
App.Test now loops to read interim 1xx responses, discards interim bodies, returns final response; introduces httpReadResponse var for test overriding. |
Tests & helpersctx_test.go, app_test.go |
Adds Test_Ctx_SendEarlyHints and three App.Test edge-case tests; adds test helpers errorReadCloser, doubleCloseBody, test errors; tests override httpReadResponse. |
Sequence Diagram(s)
sequenceDiagram
autonumber
participant Client
participant TestClient as App.Test
participant App
participant Handler
participant Res
participant Fasthttp
Client->>TestClient: raw request (test harness)
TestClient->>App: deliver request
App->>Handler: invoke route
Handler->>Res: SendEarlyHints([]Link)
Res->>Fasthttp: append Link headers and call EarlyHints()
Fasthttp-->>Client: 103 Early Hints (Link headers) -- interim
Note right of TestClient: loop reads interim 1xx, discards body, continues
Handler->>Res: write final status & body
Res->>Fasthttp: send final response
Fasthttp-->>Client: final response (e.g., 200 OK)
Estimated code review effort
π― 3 (Moderate) | β±οΈ ~25 minutes
Assessment against linked issues
| Objective | Addressed | Explanation |
|---|---|---|
| Add Early Hints (HTTP 103) capability in Fiber (#3211) | β | |
| Provide Ctx API to send Early Hints (#3211) | β | |
| Implement via connection hijacking and manual 103 construction (#3211) | β | Implementation calls fasthttp EarlyHints() and appends Link headers; no explicit hijack/manual raw write present. |
| Ensure Early Hints are sent before final response (#3211) | β |
Assessment against linked issues: Out-of-scope changes
| Code Change | Explanation |
|---|---|
App.Test multi-response loop and httpReadResponse test wrapper (app.go) |
Test-client read-loop and test wrapper are client-side test harness changes, not required by the proposal which targeted server-side Early Hints. (app.go) |
Test helper types simulating read/close failures (app_test.go) |
Added test scaffolding (errorReadCloser, doubleCloseBody) to simulate client-side errors; these are test utilities unrelated to the original server-side proposal. (app_test.go) |
Possibly related PRs
- gofiber/fiber#3172 β Changes to
SendFilesignature overlapping with this PR'sSendFile(file string, config ...SendFile)change. - gofiber/fiber#3279 β Modifies App.Test response-reading and error handling; closely related to the App.Test loop changes here.
- gofiber/fiber#3533 β Also updates Res/Ctx/DefaultRes to use variadic
SendFileconfig; relevant to API surface changes.
Suggested reviewers
- sixcolors
- grivera64
- ReneWerner87
Poem
I tapped my paw and sent a hint,
A tiny 103βso bright, a glint!
Links raced off, the browser ran,
Final bytes arrived to finish the plan.
π₯π
[!TIP]
π Remote MCP (Model Context Protocol) integration is now available!
Pro plan users can now connect to remote MCP servers from the Integrations page. Connect with popular remote MCPs such as Notion and Linear to add more context to your reviews and chats.
β¨ Finishing Touches
- [ ] π Generate Docstrings
π§ͺ Generate unit tests
- [ ] Create PR with unit tests
- [ ] Post copyable unit tests in a comment
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
πͺ§ Tips
Chat
There are 3 ways to chat with CodeRabbit:
- Review comments: Directly reply to a review comment made by CodeRabbit. Example:
-
I pushed a fix in commit <commit_id>, please review it. -
Open a follow-up GitHub issue for this discussion.
-
- Files and specific lines of code (under the "Files changed" tab): Tag
@coderabbitaiin a new review comment at the desired location with your query. - PR comments: Tag
@coderabbitaiin a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:-
@coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase. -
@coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
-
Support
Need help? Create a ticket on our support page for assistance with any issues or questions.
CodeRabbit Commands (Invoked using PR/Issue comments)
Type @coderabbitai help to get the list of available commands.
Other keywords and placeholders
- Add
@coderabbitai ignoreanywhere in the PR description to prevent this PR from being reviewed. - Add
@coderabbitai summaryto generate the high-level summary at a specific location in the PR description. - Add
@coderabbitaianywhere in the PR title to generate the title automatically.
CodeRabbit Configuration File (.coderabbit.yaml)
- You can programmatically configure CodeRabbit by adding a
.coderabbit.yamlfile to the root of your repository. - Please see the configuration documentation for more information.
- If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation:
# yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json
Status, Documentation and Community
- Visit our Status Page to check the current availability of CodeRabbit.
- Visit our Documentation for detailed information on how to use CodeRabbit.
- Join our Discord Community to get help, request features, and share feedback.
- Follow us on X/Twitter for updates and announcements.
I can't figure out why the tests are failing. Something not quite right with Fasthttp
I can't figure out why the tests are failing. Something not quite right with Fasthttp
You have a race condition in the SendEarlyHints test:
=== Failed
=== FAIL: . Test_Ctx_SendEarlyHints (0.00s)
panic: runtime error: invalid memory address or nil pointer dereference [recovered]
panic: runtime error: invalid memory address or nil pointer dereference
[signal SIGSEGV: segmentation violation code=0x1 addr=0x0 pc=0xcfe6f4]
goroutine 186 [running]:
testing.tRunner.func1.2({0x1097120, 0x1812bc0})
/opt/hostedtoolcache/go/1.23.9/x64/src/testing/testing.go:1632 +0x3fc
testing.tRunner.func1()
/opt/hostedtoolcache/go/1.23.9/x64/src/testing/testing.go:1635 +0x6b6
panic({0x1097120?, 0x1812bc0?})
/opt/hostedtoolcache/go/1.23.9/x64/src/runtime/panic.go:791 +0x132
github.com/valyala/fasthttp.acquireWriter(0xc0002e1208)
/home/runner/go/pkg/mod/github.com/valyala/[email protected]/server.go:2736 +0x54
github.com/valyala/fasthttp.(*RequestCtx).EarlyHints(0xc0002e1208)
/home/runner/go/pkg/mod/github.com/valyala/[email protected]/server.go:647 +0xab
github.com/gofiber/fiber/v3.(*DefaultCtx).SendEarlyHints(0xc0002f6308, {0xc000264370, 0x1, 0x4bdb1b?})
/home/runner/work/fiber/fiber/ctx.go:1577 +0x1d6
github.com/gofiber/fiber/v3.Test_Ctx_SendEarlyHints(0xc0003bb380)
/home/runner/work/fiber/fiber/ctx_test.go:3123 +0xf9
testing.tRunner(0xc0003bb380, 0x11a8b70)
/opt/hostedtoolcache/go/1.23.9/x64/src/testing/testing.go:1690 +0x227
created by testing.(*T).Run in goroutine 1
/opt/hostedtoolcache/go/1.23.9/x64/src/testing/testing.go:1743 +0x826
How can I fix it? It's not obvious where the race-condition is.
How can I fix it? It's not obvious where the race-condition is.
Seems the failure comes from Fasthttp? It happens when you call return c.fasthttp.EarlyHints()
How can I fix it? It's not obvious where the race-condition is.
I would recommend using the app.Test() function for this kind of unit test. I ran into a similar race condition when I was working on c.SendStreamWriter() and c.End(). (See Issue #3278, PR #3279, and the app.Test() docs)
Something is wrong. I changed the code.
app.Get("/earlyhints", func(c Ctx) error {
c.SendEarlyHints(hints)
c.Status(StatusBadRequest)
return nil
})
resp.StatusCode is not updating to last status code. It is retaining 103 Early Hints instead of 400 Bad Request
This is how the test should look like: https://github.com/valyala/fasthttp/blob/master/server_test.go#L1922
Codecov Report
:x: Patch coverage is 88.46154% with 3 lines in your changes missing coverage. Please review.
:white_check_mark: Project coverage is 91.81%. Comparing base (ac909e8) to head (03a8a25).
:warning: Report is 3 commits behind head on main.
| Files with missing lines | Patch % | Lines |
|---|---|---|
| res.go | 62.50% | 2 Missing and 1 partial :warning: |
Additional details and impacted files
@@ Coverage Diff @@
## main #3483 +/- ##
==========================================
- Coverage 91.81% 91.81% -0.01%
==========================================
Files 114 114
Lines 11498 11517 +19
==========================================
+ Hits 10557 10574 +17
- Misses 681 682 +1
- Partials 260 261 +1
| Flag | Coverage Ξ | |
|---|---|---|
| unittests | 91.81% <88.46%> (-0.01%) |
:arrow_down: |
Flags with carried forward coverage won't be shown. Click here to find out more.
:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.
:rocket: New features to boost your workflow:
- :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
Something is wrong. I changed the code.
app.Get("/earlyhints", func(c Ctx) error { c.SendEarlyHints(hints) c.Status(StatusBadRequest) return nil })
resp.StatusCodeis not updating to last status code. It is retaining103 Early Hintsinstead of400 Bad RequestThis is how the test should look like: https://github.com/valyala/fasthttp/blob/master/server_test.go#L1922
Can you try c.SendStatus(StatusBadRequest)? Maybe that's causing an issue, though I would also expect c.Status() to also work.
Didn't work
I tested it locally, and I can confirm that c.SendStatus() doesn't report StatusBadRequest properly either.
My thoughts are that one of the following is happening:
- The
app.Test()method only parses the EarlyHint, but doesn't parse the rest of the response.
For example, the HTTP response:
HTTP/1.1 103 Early Hints
Link: <https://cdn.com>; rel=preload; as=script
HTTP/1.1 200 OK
Content-Length: 0
Link: <https://cdn.com>; rel=preload; as=script
Could be parsed as only:
HTTP/1.1 103 Early Hints
Link: <https://cdn.com>; rel=preload; as=script
Possibly ignoring the rest of the response, or
- There is some bug with
SendEarlyHints(), where it only sends the hints and closes the connection.
If this is working properly locally, I believe that it's #1.
I did some local testing, and indeed it's case #1. I added a panic within app.Test() to test if this happens:
// Check if anything is left in buffer
if buffer.Buffered() > 0 {
remainingBytes, _ := buffer.Peek(buffer.Buffered())
panic("BUFFER NOT EMPTY, REMAINING BYTES:\n\n" + string(remainingBytes))
}
And we indeed got a panic when running the test:
--- FAIL: Test_Ctx_SendEarlyHints (0.00s)
panic: BUFFER NOT EMPTY, REMAINING BYTES:
HTTP/1.1 400 Bad Request
Date: Tue, 27 May 2025 05:49:16 GMT
Content-Type: text/plain; charset=utf-8
Content-Length: 11
Link: <https://cdn.com>; rel=preload; as=script
Bad Request [recovered]
panic: BUFFER NOT EMPTY, REMAINING BYTES:
HTTP/1.1 400 Bad Request
Date: Tue, 27 May 2025 05:49:16 GMT
Content-Type: text/plain; charset=utf-8
Content-Length: 11
Link: <https://cdn.com>; rel=preload; as=script
Bad Request
To properly test this feature, we would need to modify the app.Test() method (once again π) to ensure that we read the entire response as a client would.
I believe HTTP/1.1 clients are meant to ignore 1xx status codes.
Only HTTP/2+ clients know how to properly interpret it.
I believe HTTP/1.1 clients are meant to ignore 1xx status codes.
Only HTTP/2+ clients know how to properly interpret it.
@pjebs Do you know how a HTTP/2+ client would read in the request?
For example, should:
HTTP/1.1 103 Early Hints
Link: <https://cdn.com>; rel=preload; as=script
HTTP/1.1 400 Bad Request
Content-Length: 0
Link: <https://cdn.com>; rel=preload; as=script
be read as:
HTTP/1.1 400 Bad Request
Content-Length: 0
Link: <https://cdn.com>; rel=preload; as=script
Or would the client read it in some sort of other way? If the above is valid, then I have an idea of a (hacky) solution to get this to work within app.Test().
As of for actual clients, I believe that some sort of proxy that upgrades HTTP/1.1 responses into HTTP/2+ responses is necessary for production uses of Early Hints. (We should include this detail in the docs/ for this feature).
Fiber is a http 1.1 server (and client). It should simply ignore interpreting the early hints and associated headers etc until it encounters a 2xx,3xx, 4xx or 5xx I believe.
A http 2 client will properly interpret the early hints. But the final status code will not be the early hints.
But for testing the server, we still need to verify that a 103 was first sent and then the 400
@pjebs thx for the implementation , check my hints
Implementation is stuck right now
because of the problem with http2? if fiber sits behind a reverse proxy and communicates via http2 then the feature should work if not, then only no speed is saved or ?
The issue is with the Test function and it's population of Response object's status.
Also potentially fiber.Client object.
Also, a way to read the Conn data sequentially (only for unit test purposes)
is it not possible to test it like in fasthttp ? https://github.com/valyala/fasthttp/pull/1996/files#diff-53170cac6baf0bf02f3aaa3994259ea4cffaaf2498e768fb7e03cf9f752af96c
or with an inmemory listener ? https://github.com/valyala/fasthttp/blob/46ae933953a610c389f33c1058b8c2b901113e55/fasthttputil/inmemory_listener.go#L16-L30
maybe we should provide another test method that uses buffers so that streaming etc. can be tested
maybe we should provide another test method that uses buffers so that streaming etc. can be tested
@ReneWerner87 c.SendStreamWriter() is already supported with the currentapp.Test() configuration.
From my own testing, it is possible to modify our existing app.Test() implementation to work with c.SendEarlyHints() by doing the following:
-
In
app.Test(), the entire response is located in the bufferbuffer := bufio.NewReader(&conn.w). We currently read the first response inside of this buffer and discard the rest of the buffer. -
Based on how we want to implement support for EarlyHints, we can:
- Use a loop to continue trying to read from buffer for: a. A non 1XX response code (if we get any 1XX code, discard that portion of the buffer and try reading the next response) b. A non 103 response code (if we get EarlyHints, discard this portion of the buffer and try reading the next non 103 response) c. The last HTTP response (if we have a non-empty buffer, discard the currently read response and get the next one)
- Or, Track all HTTP responses in a slice (
[]*http.Response) if there are multiple responses in the output of the handler.
I have tried 1.b, and it seems to work for the unit test with c.Status(StatusBadRequest) after calling c.SendEarlyHints() above, so any of the above should help unblock @pjebs for this PR.
A similar update for the above could be made for fiber.Client, though since it's tightly coupled with fasthttp's client in execFunc(), changes might need to made upstream first if it doesn't already work. We should probably test what happens with the current implementation.
@pjebs any progress ?
It's blocked by issues in Fiber
Ok, i will test @grivera64 way for the tests
@grivera64 you mean like this ?
Patch
diff --git a/app.go b/app.go
index 75d83fd2dcbe7e916067b65a1f1a279ea0f48662..4229cabd3a95bdadf62957613a06b0e9313418cc 100644
--- a/app.go
+++ b/app.go
@@ -1094,60 +1094,74 @@ func (app *App) Test(req *http.Request, config ...TestConfig) (*http.Response, e
channel <- app.server.ServeConn(conn)
returned = true
}()
// Wait for callback
if cfg.Timeout > 0 {
// With timeout
select {
case err = <-channel:
case <-time.After(cfg.Timeout):
conn.Close() //nolint:errcheck // It is fine to ignore the error here
if cfg.FailOnTimeout {
return nil, os.ErrDeadlineExceeded
}
}
} else {
// Without timeout
err = <-channel
}
// Check for errors
if err != nil && !errors.Is(err, fasthttp.ErrGetOnly) && !errors.Is(err, errTestConnClosed) {
return nil, err
}
- // Read response
+ // Read response(s)
buffer := bufio.NewReader(&conn.w)
- // Convert raw http response to *http.Response
- res, err := http.ReadResponse(buffer, req)
- if err != nil {
- if errors.Is(err, io.ErrUnexpectedEOF) {
- return nil, ErrTestGotEmptyResponse
+ var res *http.Response
+ for {
+ // Convert raw http response to *http.Response
+ res, err = http.ReadResponse(buffer, req)
+ if err != nil {
+ if errors.Is(err, io.ErrUnexpectedEOF) {
+ return nil, ErrTestGotEmptyResponse
+ }
+ return nil, fmt.Errorf("failed to read response: %w", err)
+ }
+
+ // Break if this response is non-1xx or there are no more responses
+ if res.StatusCode >= http.StatusOK || buffer.Buffered() == 0 {
+ break
+ }
+
+ // Discard interim response body before reading the next one
+ if res.Body != nil {
+ _, _ = io.Copy(io.Discard, res.Body)
+ res.Body.Close()
}
- return nil, fmt.Errorf("failed to read response: %w", err)
}
return res, nil
}
type disableLogger struct{}
func (*disableLogger) Printf(string, ...any) {
}
func (app *App) init() *App {
// lock application
app.mutex.Lock()
// Initialize Services when needed,
// panics if there is an error starting them.
app.initServices()
// Only load templates if a view engine is specified
if app.config.Views != nil {
if err := app.config.Views.Load(); err != nil {
log.Warnf("failed to load views: %v", err)
}
}
diff --git a/app_test.go b/app_test.go
index 60232160e029885bd1215ee53e3eb05dbf40ddce..9d887d90518d801d5169bddcae0cdd64db9f5243 100644
--- a/app_test.go
+++ b/app_test.go
@@ -1737,50 +1737,71 @@ func Test_App_Test_timeout_empty_response(t *testing.T) {
return nil
})
_, err := app.Test(httptest.NewRequest(MethodGet, "/", nil), TestConfig{
Timeout: 100 * time.Millisecond,
FailOnTimeout: false,
})
require.ErrorIs(t, err, ErrTestGotEmptyResponse)
}
func Test_App_Test_drop_empty_response(t *testing.T) {
t.Parallel()
app := New()
app.Get("/", func(c Ctx) error {
return c.Drop()
})
_, err := app.Test(httptest.NewRequest(MethodGet, "/", nil), TestConfig{
Timeout: 0,
FailOnTimeout: false,
})
require.ErrorIs(t, err, ErrTestGotEmptyResponse)
}
+func Test_App_Test_EarlyHints(t *testing.T) {
+ t.Parallel()
+
+ app := New()
+ hints := []string{"<https://cdn.com>; rel=preload; as=script"}
+ app.Get("/early", func(c Ctx) error {
+ c.SendEarlyHints(hints)
+ return c.Status(StatusOK).SendString("done")
+ })
+
+ req := httptest.NewRequest(MethodGet, "/early", nil)
+ resp, err := app.Test(req)
+
+ require.NoError(t, err)
+ require.Equal(t, StatusOK, resp.StatusCode)
+ require.Equal(t, hints, resp.Header["Link"])
+ body, err := io.ReadAll(resp.Body)
+ require.NoError(t, err)
+ require.Equal(t, "done", string(body))
+}
+
func Test_App_SetTLSHandler(t *testing.T) {
t.Parallel()
tlsHandler := &TLSHandler{clientHelloInfo: &tls.ClientHelloInfo{
ServerName: "example.golang",
}}
app := New()
app.SetTLSHandler(tlsHandler)
c := app.AcquireCtx(&fasthttp.RequestCtx{})
defer app.ReleaseCtx(c)
require.Equal(t, "example.golang", c.ClientHelloInfo().ServerName)
}
func Test_App_AddCustomRequestMethod(t *testing.T) {
t.Parallel()
methods := append(DefaultMethods, "TEST") //nolint:gocritic // We want a new slice here
app := New(Config{
RequestMethods: methods,
})
appMethods := app.config.RequestMethods
// method name is always uppercase - https://datatracker.ietf.org/doc/html/rfc7231#section-4.1
require.Equal(t, len(app.stack), len(appMethods))
diff --git a/ctx_test.go b/ctx_test.go
index 0510f7b47e27f241df7a16411e8447e8bf2b57a0..0de5eea8f4253e3961d8c8922b4b31480143286b 100644
--- a/ctx_test.go
+++ b/ctx_test.go
@@ -3467,62 +3467,62 @@ func Test_Ctx_Download(t *testing.T) {
}()
expect, err := io.ReadAll(f)
require.NoError(t, err)
require.Equal(t, expect, c.Response().Body())
require.Equal(t, `attachment; filename="Awesome+File%21"`, string(c.Response().Header.Peek(HeaderContentDisposition)))
require.NoError(t, c.Res().Download("ctx.go"))
require.Equal(t, `attachment; filename="ctx.go"`, string(c.Response().Header.Peek(HeaderContentDisposition)))
require.NoError(t, c.Download("ctx.go", "ΡΠ°ΠΉΠ».txt"))
header := string(c.Response().Header.Peek(HeaderContentDisposition))
require.Contains(t, header, `filename="ΡΠ°ΠΉΠ».txt"`)
require.Contains(t, header, `filename*=UTF-8''%D1%84%D0%B0%D0%B9%D0%BB.txt`)
}
// go test -race -run Test_Ctx_SendEarlyHints
func Test_Ctx_SendEarlyHints(t *testing.T) {
t.Parallel()
app := New()
hints := []string{"<https://cdn.com>; rel=preload; as=script"}
app.Get("/earlyhints", func(c Ctx) error {
c.SendEarlyHints(hints)
c.Status(StatusBadRequest)
- return nil
+ return c.SendString("fail")
})
req := httptest.NewRequest(MethodGet, "/earlyhints", nil)
resp, err := app.Test(req)
- fmt.Println(resp.StatusCode)
- fmt.Println(resp.Header)
-
require.NoError(t, err, "app.Test(req)")
- require.Equal(t, StatusEarlyHints, resp.StatusCode, "Status code")
+ require.Equal(t, StatusBadRequest, resp.StatusCode, "Status code")
require.Equal(t, hints, resp.Header["Link"], "Link header")
+ body, err := io.ReadAll(resp.Body)
+ require.NoError(t, err)
+ require.Equal(t, "fail", string(body))
}
// go test -race -run Test_Ctx_SendFile
func Test_Ctx_SendFile(t *testing.T) {
t.Parallel()
app := New()
// fetch file content
f, err := os.Open("./ctx.go")
require.NoError(t, err)
defer func() {
require.NoError(t, f.Close())
}()
expectFileContent, err := io.ReadAll(f)
require.NoError(t, err)
// fetch file info for the not modified test case
fI, err := os.Stat("./ctx.go")
require.NoError(t, err)
// simple test case
c := app.AcquireCtx(&fasthttp.RequestCtx{})
err = c.SendFile("ctx.go")
// check expectation
require.NoError(t, err)
require.Equal(t, expectFileContent, c.Response().Body())
@grivera64 you mean like this ?
Patch
diff --git a/app.go b/app.go index 75d83fd2dcbe7e916067b65a1f1a279ea0f48662..4229cabd3a95bdadf62957613a06b0e9313418cc 100644 --- a/app.go +++ b/app.go @@ -1094,60 +1094,74 @@ func (app *App) Test(req *http.Request, config ...TestConfig) (*http.Response, e channel <- app.server.ServeConn(conn) returned = true }() // Wait for callback if cfg.Timeout > 0 { // With timeout select { case err = <-channel: case <-time.After(cfg.Timeout): conn.Close() //nolint:errcheck // It is fine to ignore the error here if cfg.FailOnTimeout { return nil, os.ErrDeadlineExceeded } } } else { // Without timeout err = <-channel } // Check for errors if err != nil && !errors.Is(err, fasthttp.ErrGetOnly) && !errors.Is(err, errTestConnClosed) { return nil, err } - // Read response + // Read response(s) buffer := bufio.NewReader(&conn.w) - // Convert raw http response to *http.Response - res, err := http.ReadResponse(buffer, req) - if err != nil { - if errors.Is(err, io.ErrUnexpectedEOF) { - return nil, ErrTestGotEmptyResponse + var res *http.Response + for { + // Convert raw http response to *http.Response + res, err = http.ReadResponse(buffer, req) + if err != nil { + if errors.Is(err, io.ErrUnexpectedEOF) { + return nil, ErrTestGotEmptyResponse + } + return nil, fmt.Errorf("failed to read response: %w", err) + } + + // Break if this response is non-1xx or there are no more responses + if res.StatusCode >= http.StatusOK || buffer.Buffered() == 0 { + break + } + + // Discard interim response body before reading the next one + if res.Body != nil { + _, _ = io.Copy(io.Discard, res.Body) + res.Body.Close() } - return nil, fmt.Errorf("failed to read response: %w", err) } return res, nil } type disableLogger struct{} func (*disableLogger) Printf(string, ...any) { } func (app *App) init() *App { // lock application app.mutex.Lock() // Initialize Services when needed, // panics if there is an error starting them. app.initServices() // Only load templates if a view engine is specified if app.config.Views != nil { if err := app.config.Views.Load(); err != nil { log.Warnf("failed to load views: %v", err) } } diff --git a/app_test.go b/app_test.go index 60232160e029885bd1215ee53e3eb05dbf40ddce..9d887d90518d801d5169bddcae0cdd64db9f5243 100644 --- a/app_test.go +++ b/app_test.go @@ -1737,50 +1737,71 @@ func Test_App_Test_timeout_empty_response(t *testing.T) { return nil }) _, err := app.Test(httptest.NewRequest(MethodGet, "/", nil), TestConfig{ Timeout: 100 * time.Millisecond, FailOnTimeout: false, }) require.ErrorIs(t, err, ErrTestGotEmptyResponse) } func Test_App_Test_drop_empty_response(t *testing.T) { t.Parallel() app := New() app.Get("/", func(c Ctx) error { return c.Drop() }) _, err := app.Test(httptest.NewRequest(MethodGet, "/", nil), TestConfig{ Timeout: 0, FailOnTimeout: false, }) require.ErrorIs(t, err, ErrTestGotEmptyResponse) } +func Test_App_Test_EarlyHints(t *testing.T) { + t.Parallel() + + app := New() + hints := []string{"<https://cdn.com>; rel=preload; as=script"} + app.Get("/early", func(c Ctx) error { + c.SendEarlyHints(hints) + return c.Status(StatusOK).SendString("done") + }) + + req := httptest.NewRequest(MethodGet, "/early", nil) + resp, err := app.Test(req) + + require.NoError(t, err) + require.Equal(t, StatusOK, resp.StatusCode) + require.Equal(t, hints, resp.Header["Link"]) + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, "done", string(body)) +} + func Test_App_SetTLSHandler(t *testing.T) { t.Parallel() tlsHandler := &TLSHandler{clientHelloInfo: &tls.ClientHelloInfo{ ServerName: "example.golang", }} app := New() app.SetTLSHandler(tlsHandler) c := app.AcquireCtx(&fasthttp.RequestCtx{}) defer app.ReleaseCtx(c) require.Equal(t, "example.golang", c.ClientHelloInfo().ServerName) } func Test_App_AddCustomRequestMethod(t *testing.T) { t.Parallel() methods := append(DefaultMethods, "TEST") //nolint:gocritic // We want a new slice here app := New(Config{ RequestMethods: methods, }) appMethods := app.config.RequestMethods // method name is always uppercase - https://datatracker.ietf.org/doc/html/rfc7231#section-4.1 require.Equal(t, len(app.stack), len(appMethods)) diff --git a/ctx_test.go b/ctx_test.go index 0510f7b47e27f241df7a16411e8447e8bf2b57a0..0de5eea8f4253e3961d8c8922b4b31480143286b 100644 --- a/ctx_test.go +++ b/ctx_test.go @@ -3467,62 +3467,62 @@ func Test_Ctx_Download(t *testing.T) { }() expect, err := io.ReadAll(f) require.NoError(t, err) require.Equal(t, expect, c.Response().Body()) require.Equal(t, `attachment; filename="Awesome+File%21"`, string(c.Response().Header.Peek(HeaderContentDisposition))) require.NoError(t, c.Res().Download("ctx.go")) require.Equal(t, `attachment; filename="ctx.go"`, string(c.Response().Header.Peek(HeaderContentDisposition))) require.NoError(t, c.Download("ctx.go", "ΡΠ°ΠΉΠ».txt")) header := string(c.Response().Header.Peek(HeaderContentDisposition)) require.Contains(t, header, `filename="ΡΠ°ΠΉΠ».txt"`) require.Contains(t, header, `filename*=UTF-8''%D1%84%D0%B0%D0%B9%D0%BB.txt`) } // go test -race -run Test_Ctx_SendEarlyHints func Test_Ctx_SendEarlyHints(t *testing.T) { t.Parallel() app := New() hints := []string{"<https://cdn.com>; rel=preload; as=script"} app.Get("/earlyhints", func(c Ctx) error { c.SendEarlyHints(hints) c.Status(StatusBadRequest) - return nil + return c.SendString("fail") }) req := httptest.NewRequest(MethodGet, "/earlyhints", nil) resp, err := app.Test(req) - fmt.Println(resp.StatusCode) - fmt.Println(resp.Header) - require.NoError(t, err, "app.Test(req)") - require.Equal(t, StatusEarlyHints, resp.StatusCode, "Status code") + require.Equal(t, StatusBadRequest, resp.StatusCode, "Status code") require.Equal(t, hints, resp.Header["Link"], "Link header") + body, err := io.ReadAll(resp.Body) + require.NoError(t, err) + require.Equal(t, "fail", string(body)) } // go test -race -run Test_Ctx_SendFile func Test_Ctx_SendFile(t *testing.T) { t.Parallel() app := New() // fetch file content f, err := os.Open("./ctx.go") require.NoError(t, err) defer func() { require.NoError(t, f.Close()) }() expectFileContent, err := io.ReadAll(f) require.NoError(t, err) // fetch file info for the not modified test case fI, err := os.Stat("./ctx.go") require.NoError(t, err) // simple test case c := app.AcquireCtx(&fasthttp.RequestCtx{}) err = c.SendFile("ctx.go") // check expectation require.NoError(t, err) require.Equal(t, expectFileContent, c.Response().Body())
@ReneWerner87 Yes, the patch LGTM π
We could also consider having a separate Test method that returns the full stream of responses as a slice, but I am not sure if this is necessary.
If that's not needed, then I believe this is all we need to merge the PR to main.
@pjebs can you check my patch
Where is the patch?