fiber icon indicating copy to clipboard operation
fiber copied to clipboard

πŸ”₯ feat: Support for SendEarlyHints

Open pjebs opened this issue 7 months ago β€’ 23 comments

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

pjebs avatar May 27 '25 01:05 pjebs

[!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 changes
ctx_interface_gen.go
Adds SendEarlyHints(hints []string) error; changes SendFile(file string) error β†’ SendFile(file string, config ...SendFile) error.
Res API & impl
res_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 client
app.go
App.Test now loops to read interim 1xx responses, discards interim bodies, returns final response; introduces httpReadResponse var for test overriding.
Tests & helpers
ctx_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 SendFile signature overlapping with this PR's SendFile(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 SendFile config; 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.

❀️ Share
πŸͺ§ 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 @coderabbitai in a new review comment at the desired location with your query.
  • PR comments: Tag @coderabbitai in 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 ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file 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.

coderabbitai[bot] avatar May 27 '25 01:05 coderabbitai[bot]

I can't figure out why the tests are failing. Something not quite right with Fasthttp

pjebs avatar May 27 '25 01:05 pjebs

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

gaby avatar May 27 '25 01:05 gaby

How can I fix it? It's not obvious where the race-condition is.

pjebs avatar May 27 '25 01:05 pjebs

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()

gaby avatar May 27 '25 02:05 gaby

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)

grivera64 avatar May 27 '25 02:05 grivera64

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

pjebs avatar May 27 '25 04:05 pjebs

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.

codecov[bot] avatar May 27 '25 04:05 codecov[bot]

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

Can you try c.SendStatus(StatusBadRequest)? Maybe that's causing an issue, though I would also expect c.Status() to also work.

grivera64 avatar May 27 '25 05:05 grivera64

Didn't work

pjebs avatar May 27 '25 05:05 pjebs

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:

  1. 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

  1. 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.

grivera64 avatar May 27 '25 05:05 grivera64

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.

grivera64 avatar May 27 '25 05:05 grivera64

I believe HTTP/1.1 clients are meant to ignore 1xx status codes.

Only HTTP/2+ clients know how to properly interpret it.

pjebs avatar May 27 '25 05:05 pjebs

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).

grivera64 avatar May 27 '25 06:05 grivera64

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 avatar May 27 '25 06:05 pjebs

@pjebs thx for the implementation , check my hints

ReneWerner87 avatar May 27 '25 09:05 ReneWerner87

Implementation is stuck right now

pjebs avatar May 27 '25 12:05 pjebs

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 ?

ReneWerner87 avatar May 27 '25 12:05 ReneWerner87

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)

pjebs avatar May 27 '25 12:05 pjebs

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

ReneWerner87 avatar May 27 '25 12:05 ReneWerner87

maybe we should provide another test method that uses buffers so that streaming etc. can be tested

ReneWerner87 avatar May 27 '25 12:05 ReneWerner87

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 buffer buffer := 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:

  1. 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)
  2. 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.

grivera64 avatar May 27 '25 22:05 grivera64

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.

grivera64 avatar May 27 '25 22:05 grivera64

@pjebs any progress ?

ReneWerner87 avatar Jun 30 '25 07:06 ReneWerner87

It's blocked by issues in Fiber

pjebs avatar Jun 30 '25 14:06 pjebs

Ok, i will test @grivera64 way for the tests

ReneWerner87 avatar Jun 30 '25 16:06 ReneWerner87

@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 avatar Jul 01 '25 09:07 ReneWerner87

@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.

grivera64 avatar Jul 02 '25 01:07 grivera64

@pjebs can you check my patch

ReneWerner87 avatar Jul 02 '25 11:07 ReneWerner87

Where is the patch?

pjebs avatar Jul 04 '25 02:07 pjebs