Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

proposal: net/http/httptest: add support for 1XX responses #56346

Open
dunglas opened this issue Oct 20, 2022 · 9 comments · May be fixed by #56151
Open

proposal: net/http/httptest: add support for 1XX responses #56346

dunglas opened this issue Oct 20, 2022 · 9 comments · May be fixed by #56151
Labels
Proposal
Milestone
Proposal

Comments

@dunglas
Copy link
Contributor

dunglas commented Oct 20, 2022 •
edited

There is currently no way to test 1XX responses with the httptest package.

Support for 1XX responses has been added to the built-in HTTP server and reverse proxy in Go 1.19 (#42597, #53164).

I propose to add support for inspecting 1XX responses with the test package by integrating the httptrace package with it (as we've done for the reverse proxy).

Regarding the API, I suggest implementing the same API as when using the non-test client:

func Test1xx(t *testing.T) {
	trace := &httptrace.ClientTrace{
		Got100Continue: func() {
			// do something
		},
		Got1xxResponse: func(code int, header textproto.MIMEHeader) error {
			// do something

			return nil
		},
	}

	r, _ := http.NewRequest("GET", "http://example.com", nil)
	rw := httptest.NewRecorder()
	rw.ClientTrace = trace
	func(rw http.ResponseWriter, _ *http.Request) {
		rw.Header().Add("Link", "</image.png>; rel=preload; as=image")
		rw.WriteHeader(http.StatusEarlyHints)

		// ...
	}(rw, r)

       // ...
}

Proposal implementation: #56151
@gopherbot gopherbot added this to the Proposal milestone Oct 20, 2022
@ianlancetaylor

This comment was marked as resolved.

Sign in to view
@dunglas

This comment was marked as resolved.

Sign in to view
kisunji added a commit to hashicorp/consul that referenced this issue Oct 24, 2022
@kisunji
Remove invalid 1xx HTTP codes
e78711a 
These tests started failing in go1.19, presumably due to
support for valid 1xx responses being added.

golang/go#56346
kisunji added a commit to hashicorp/consul that referenced this issue Oct 24, 2022
@kisunji
Remove invalid 1xx HTTP codes
6d6ceaf 
These tests started failing in go1.19, presumably due to
support for valid 1xx responses being added.

golang/go#56346
kisunji added a commit to hashicorp/consul that referenced this issue Oct 24, 2022
@kisunji
Remove invalid 1xx HTTP codes
7f21994 
These tests started failing in go1.19, presumably due to
support for valid 1xx responses being added.

golang/go#56346
kisunji added a commit to hashicorp/consul that referenced this issue Oct 24, 2022
@kisunji
Remove invalid 1xx HTTP codes
e36f869 
These tests started failing in go1.19, presumably due to
support for valid 1xx responses being added.

golang/go#56346
kisunji added a commit to hashicorp/consul that referenced this issue Oct 24, 2022
@kisunji
Remove invalid 1xx HTTP codes
e953632 
These tests started failing in go1.19, presumably due to
support for valid 1xx responses being added.

golang/go#56346
kisunji added a commit to hashicorp/consul that referenced this issue Oct 24, 2022
@kisunji
Remove invalid 1xx HTTP codes
9f0ed81 
These tests started failing in go1.19, presumably due to
support for valid 1xx responses being added.

golang/go#56346
@dunglas dunglas linked a pull request Nov 23, 2022 that will close this issue
Open
@dunglas
Copy link
Contributor Author

dunglas commented Nov 23, 2022 •
edited

Updated API proposal following https://go-review.googlesource.com/c/go/+/442215/1#message-b8b8e699ed0237031dbfec5e34b8adf1bf8a31d8:

// InformationalResponse is an HTTP response sent with a [1xx status code].
//
// [1xx status code]: https://httpwg.org/specs/rfc9110.html#status.1xx
type InformationalResponse struct {
	// Code is the 1xx HTTP response code of this informational response.
	Code int

	// Header contains the headers of this informational response.
	Header http.Header
}

type ResponseRecorder struct {
	// Informational HTTP responses (1xx status code) sent before the main response.
	InformationalResponses []InformationalResponse

	// ...
}

@gopherbot
Copy link

Change https://go.dev/cl/442215 mentions this issue: net/http/httptest: add support for 1XX responses

@neild
Copy link
Contributor

neild commented May 17, 2023

The updated proposal in #56346 (comment) seems reasonable to me.

@Acconut
Copy link

Acconut commented Aug 24, 2023

Thank you for working on this! It would be helpful to have support for informational responses and the proposed API looks good to me. In https://go-review.googlesource.com/c/go/+/442215, @neild mentioned that an accepted proposal is needed to proceed here. I am new to contributing to Go, but what are the steps to continue with the proposal process? Is a design document necessary as mentioned in https://github.com/golang/proposal#readme?

@Acconut
Copy link

Acconut commented Aug 24, 2023

@dunglas Thank you for implementing the proposed API. To try it out and use it today, I pulled the response recorder and your PR into a separate package: https://github.com/Acconut/go-httptest-recorder I hope this is OK and helps to experiment with the new feature.

@dunglas
Copy link
Contributor Author

dunglas commented Aug 24, 2023

@Acconut thanks for creating the package. It works just fine for me, although I hope this patch lands in the stdlib at some point 🙂

@mitar
Copy link
Contributor

mitar commented Oct 27, 2023

@neild @ianlancetaylor What is the process to get this from Incoming to Active proposal?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Proposal
Projects
Proposals
Status: Incoming
Milestone
Proposal
Development

Successfully merging a pull request may close this issue.

net/http/httptest: add support for 1XX responses dunglas/go
6 participants
@neild @dunglas @mitar @Acconut @ianlancetaylor @gopherbot