From f0b508100d688f25d087e7e673d7519c540bd061 Mon Sep 17 00:00:00 2001 From: Oleg Bespalov Date: Fri, 1 Dec 2023 09:21:10 +0100 Subject: [PATCH 1/5] Fixes minor lib/netext linter issues --- lib/netext/dialer.go | 2 +- lib/netext/httpext/error_codes_syscall_posix.go | 2 +- lib/netext/httpext/error_codes_test.go | 8 ++++++-- lib/netext/httpext/request_test.go | 5 ++++- lib/netext/httpext/tracer.go | 10 +++++----- lib/netext/httpext/tracer_test.go | 2 +- lib/netext/httpext/transport.go | 1 + lib/netext/httpext/transport_test.go | 4 ++-- lib/netext/resolver_test.go | 3 ++- lib/netext/tls.go | 7 ++++++- 10 files changed, 29 insertions(+), 15 deletions(-) diff --git a/lib/netext/dialer.go b/lib/netext/dialer.go index 01f776904daa..e9db26fb2b60 100644 --- a/lib/netext/dialer.go +++ b/lib/netext/dialer.go @@ -205,7 +205,7 @@ func (d *Dialer) getConfiguredHost(addr, host, port string) (*types.Host, error) return &newRemote, nil } - return nil, nil + return nil, nil //nolint:nilnil } // NetTrail contains information about the exchanged data size and length of a diff --git a/lib/netext/httpext/error_codes_syscall_posix.go b/lib/netext/httpext/error_codes_syscall_posix.go index b0a66f83ce22..09bc8b712b15 100644 --- a/lib/netext/httpext/error_codes_syscall_posix.go +++ b/lib/netext/httpext/error_codes_syscall_posix.go @@ -8,6 +8,6 @@ import ( "os" ) -func getOSSyscallErrorCode(e *net.OpError, se *os.SyscallError) (errCode, string) { +func getOSSyscallErrorCode(_ *net.OpError, _ *os.SyscallError) (errCode, string) { return 0, "" } diff --git a/lib/netext/httpext/error_codes_test.go b/lib/netext/httpext/error_codes_test.go index 9272d680efac..8367c1a5903a 100644 --- a/lib/netext/httpext/error_codes_test.go +++ b/lib/netext/httpext/error_codes_test.go @@ -184,7 +184,11 @@ func TestHTTP2StreamError(t *testing.T) { rw.Header().Set("Content-Length", "100000") rw.WriteHeader(http.StatusOK) - rw.(http.Flusher).Flush() + f, ok := rw.(http.Flusher) + if !ok { + panic("expected http.ResponseWriter to be http.Flusher") + } + f.Flush() time.Sleep(time.Millisecond * 2) panic("expected internal error") }) @@ -376,7 +380,7 @@ func getHTTP2ServerWithCustomConnContext(t *testing.T) *httpmultibin.HTTPMultiBi Replacer: strings.NewReplacer( "HTTP2BIN_IP_URL", http2Srv.URL, "HTTP2BIN_DOMAIN", http2Domain, - "HTTP2BIN_URL", fmt.Sprintf("https://%s:%s", http2Domain, http2URL.Port()), + "HTTP2BIN_URL", fmt.Sprintf("https://%s", net.JoinHostPort(http2Domain, http2URL.Port())), "HTTP2BIN_IP", http2IP.String(), "HTTP2BIN_PORT", http2URL.Port(), ), diff --git a/lib/netext/httpext/request_test.go b/lib/netext/httpext/request_test.go index c2e5b956f779..7bb6441acb96 100644 --- a/lib/netext/httpext/request_test.go +++ b/lib/netext/httpext/request_test.go @@ -351,7 +351,10 @@ func TestTrailFailed(t *testing.T) { require.NotNil(t, res) require.Len(t, samples, 1) sample := <-samples - trail := sample.(*Trail) + + trail, ok := sample.(*Trail) + require.True(t, ok) + require.Equal(t, failed, trail.Failed) var httpReqFailedSampleValue null.Bool diff --git a/lib/netext/httpext/tracer.go b/lib/netext/httpext/tracer.go index 3af12f627024..9f8edd2aaaf4 100644 --- a/lib/netext/httpext/tracer.go +++ b/lib/netext/httpext/tracer.go @@ -40,7 +40,7 @@ type Trail struct { Samples []metrics.Sample } -// SaveSamples populates the Trail's sample slice so they're accesible via GetSamples() +// SaveSamples populates the Trail's sample slice so they're accessible via GetSamples() func (tr *Trail) SaveSamples(builtinMetrics *metrics.BuiltinMetrics, ctm *metrics.TagsAndMeta) { tr.Tags = ctm.Tags tr.Metadata = ctm.Metadata @@ -184,7 +184,7 @@ func now() int64 { // Keep in mind that GetConn won't be called if a connection // is reused though, for example when there's a redirect. // If it's called, it will be called before all other hooks. -func (t *Tracer) GetConn(hostPort string) { +func (t *Tracer) GetConn(_ string) { t.getConn = now() } @@ -194,7 +194,7 @@ func (t *Tracer) GetConn(hostPort string) { // // If the connection is reused, this won't be called. Otherwise, // it will be called after GetConn() and before ConnectDone(). -func (t *Tracer) ConnectStart(network, addr string) { +func (t *Tracer) ConnectStart(_, _ string) { // If using dual-stack dialing, it's possible to get this // multiple times, so the atomic compareAndSwap ensures // that only the first call's time is recorded @@ -210,7 +210,7 @@ func (t *Tracer) ConnectStart(network, addr string) { // If the connection is reused, this won't be called. Otherwise, // it will be called after ConnectStart() and before either // TLSHandshakeStart() (for TLS connections) or GotConn(). -func (t *Tracer) ConnectDone(network, addr string, err error) { +func (t *Tracer) ConnectDone(_, _ string, err error) { // If using dual-stack dialing, it's possible to get this // multiple times, so the atomic compareAndSwap ensures // that only the first call's time is recorded @@ -239,7 +239,7 @@ func (t *Tracer) TLSHandshakeStart() { // it will be called after TLSHandshakeStart() and before GotConn(). // If the request was cancelled, this could be called after the // RoundTrip() method has returned. -func (t *Tracer) TLSHandshakeDone(state tls.ConnectionState, err error) { +func (t *Tracer) TLSHandshakeDone(_ tls.ConnectionState, err error) { if err == nil { atomic.CompareAndSwapInt64(&t.tlsHandshakeDone, 0, now()) } diff --git a/lib/netext/httpext/tracer_test.go b/lib/netext/httpext/tracer_test.go index efec14a939d0..7fce63c6477c 100644 --- a/lib/netext/httpext/tracer_test.go +++ b/lib/netext/httpext/tracer_test.go @@ -244,7 +244,7 @@ func TestTracerError(t *testing.T) { req, err := http.NewRequest(http.MethodGet, srv.URL+"/get", nil) require.NoError(t, err) - _, err = http.DefaultTransport.RoundTrip( + _, err = http.DefaultTransport.RoundTrip( //nolint:bodyclose req.WithContext( httptrace.WithClientTrace( context.Background(), diff --git a/lib/netext/httpext/transport.go b/lib/netext/httpext/transport.go index dc6dc7ee3603..f9b49a556e8c 100644 --- a/lib/netext/httpext/transport.go +++ b/lib/netext/httpext/transport.go @@ -1,3 +1,4 @@ +// Package httpext provides extensions to the standard net/http package package httpext import ( diff --git a/lib/netext/httpext/transport_test.go b/lib/netext/httpext/transport_test.go index 261ee1fd36b4..1a0497026151 100644 --- a/lib/netext/httpext/transport_test.go +++ b/lib/netext/httpext/transport_test.go @@ -16,8 +16,8 @@ func BenchmarkMeasureAndEmitMetrics(b *testing.B) { defer cancel() samples := make(chan metrics.SampleContainer, 10) defer close(samples) - go func() { - for range samples { + go func() { // discard all metrics + for range samples { //nolint:revive } }() logger := logrus.New() diff --git a/lib/netext/resolver_test.go b/lib/netext/resolver_test.go index 56d6f0f6334e..df659c04f458 100644 --- a/lib/netext/resolver_test.go +++ b/lib/netext/resolver_test.go @@ -73,7 +73,8 @@ func TestResolver(t *testing.T) { if tc.ttl > 0 { require.IsType(t, &cacheResolver{}, r) - cr := r.(*cacheResolver) + cr, ok := r.(*cacheResolver) + assert.True(t, ok) assert.Len(t, cr.cache, 1) assert.Equal(t, tc.ttl, cr.ttl) firstLookup := cr.cache[host].lastLookup diff --git a/lib/netext/tls.go b/lib/netext/tls.go index f04607bcc6d7..8d8a9171fcfa 100644 --- a/lib/netext/tls.go +++ b/lib/netext/tls.go @@ -1,3 +1,4 @@ +// Package netext provides extensions to the standard net package package netext import ( @@ -8,7 +9,7 @@ import ( "go.k6.io/k6/lib" ) -//nolint:golint +//nolint:golint,revive,stylecheck // we want to keep these constants as they are const ( OCSP_STATUS_GOOD = "good" OCSP_STATUS_REVOKED = "revoked" @@ -30,10 +31,13 @@ const ( TLS_1_3 = "tls1.3" ) +// TLSInfo keeps TLS details type TLSInfo struct { Version string CipherSuite string } + +// OCSP keeps Online Certificate Status Protocol (OCSP) details type OCSP struct { ProducedAt int64 `json:"produced_at"` ThisUpdate int64 `json:"this_update"` @@ -43,6 +47,7 @@ type OCSP struct { Status string `json:"status"` } +// ParseTLSConnState parses tls.ConnectionState and returns TLS and OCSP details func ParseTLSConnState(tlsState *tls.ConnectionState) (TLSInfo, OCSP) { tlsInfo := TLSInfo{} switch tlsState.Version { From 68e869a8ffb40bcbea532c3a82218052fb62af1b Mon Sep 17 00:00:00 2001 From: Oleg Bespalov Date: Fri, 1 Dec 2023 09:30:57 +0100 Subject: [PATCH 2/5] Removes global var usage from tracer_test --- lib/netext/httpext/tracer_test.go | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/lib/netext/httpext/tracer_test.go b/lib/netext/httpext/tracer_test.go index 7fce63c6477c..74a129e139fd 100644 --- a/lib/netext/httpext/tracer_test.go +++ b/lib/netext/httpext/tracer_test.go @@ -176,13 +176,12 @@ func TestTracer(t *testing.T) { //nolint:tparallel type failingConn struct { net.Conn + failOnConnWrite bool } -var failOnConnWrite = false - -func (c failingConn) Write(b []byte) (int, error) { - if failOnConnWrite { - failOnConnWrite = false +func (c *failingConn) Write(b []byte) (int, error) { + if c.failOnConnWrite { + c.failOnConnWrite = false return 0, errors.New("write error") } @@ -198,9 +197,11 @@ func TestTracerNegativeHttpSendingValues(t *testing.T) { assert.True(t, ok) dialer := &net.Dialer{} + var connection *failingConn transport.DialContext = func(ctx context.Context, proto, addr string) (net.Conn, error) { conn, err := dialer.DialContext(ctx, proto, addr) - return failingConn{conn}, err + connection = &failingConn{conn, false} + return connection, err } req, err := http.NewRequest(http.MethodGet, srv.URL+"/get", nil) @@ -217,7 +218,7 @@ func TestTracerNegativeHttpSendingValues(t *testing.T) { } // make the next connection write fail - failOnConnWrite = true + connection.failOnConnWrite = true { tracer := &Tracer{} From 58b7e0894693ec901541edddf9255d5529c4aa09 Mon Sep 17 00:00:00 2001 From: Oleg Bespalov Date: Fri, 1 Dec 2023 09:34:08 +0100 Subject: [PATCH 3/5] Fixes httpext tracer linter issues --- lib/netext/httpext/tracer.go | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/lib/netext/httpext/tracer.go b/lib/netext/httpext/tracer.go index 9f8edd2aaaf4..7ac89a13df82 100644 --- a/lib/netext/httpext/tracer.go +++ b/lib/netext/httpext/tracer.go @@ -344,14 +344,15 @@ func (t *Tracer) Done() *Trail { trail.TLSHandshaking = time.Duration(tlsHandshakeDone - tlsHandshakeStart) } if wroteRequest != 0 { - if tlsHandshakeDone != 0 { + switch { + case tlsHandshakeDone != 0: // If the request was sent over TLS, we need to use // TLS Handshake Done time to calculate sending duration trail.Sending = time.Duration(wroteRequest - tlsHandshakeDone) - } else if connectDone != 0 { + case connectDone != 0: // Otherwise, use the end of the normal connection trail.Sending = time.Duration(wroteRequest - connectDone) - } else { + default: // Finally, this handles the strange HTTP/2 case where the GotConn() hook // gets called first, but with Reused=false trail.Sending = time.Duration(wroteRequest - gotConn) From 3b87e55dfb8a417c09e10c8e304f82149d877649 Mon Sep 17 00:00:00 2001 From: Oleg Bespalov Date: Fri, 1 Dec 2023 09:47:07 +0100 Subject: [PATCH 4/5] Fixes httpext compression linter issues --- lib/netext/httpext/compression.go | 52 +++++++++++++++++-------------- 1 file changed, 29 insertions(+), 23 deletions(-) diff --git a/lib/netext/httpext/compression.go b/lib/netext/httpext/compression.go index ba3b2f9a2161..6716fb4ad5ee 100644 --- a/lib/netext/httpext/compression.go +++ b/lib/netext/httpext/compression.go @@ -4,6 +4,7 @@ import ( "bytes" "compress/gzip" "compress/zlib" + "errors" "fmt" "io" "net/http" @@ -104,7 +105,7 @@ func wrapDecompressionError(err error) error { // we don't use it... maybe the code that builds the decompression readers // could also add an appropriate error-wrapper layer? for _, decErr := range &decompressionErrors { - if err == decErr { + if errors.Is(err, decErr) { return newDecompressionError(err) } } @@ -127,10 +128,8 @@ func readResponseBody( if respType == ResponseTypeNone { _, err := io.Copy(io.Discard, resp.Body) _ = resp.Body.Close() - if err != nil { - respErr = err - } - return nil, respErr + + return nil, err } rc := &readCloser{resp.Body} @@ -153,27 +152,11 @@ func readResponseBody( for i := len(contentEncodings) - 1; i >= 0; i-- { contentEncoding := strings.TrimSpace(contentEncodings[i]) if compression, err := CompressionTypeString(contentEncoding); err == nil { - var decoder io.Reader - var err error - switch compression { - case CompressionTypeDeflate: - decoder, err = zlib.NewReader(rc) - case CompressionTypeGzip: - decoder, err = gzip.NewReader(rc) - case CompressionTypeZstd: - decoder, err = zstd.NewReader(rc) - case CompressionTypeBr: - decoder = brotli.NewReader(rc) - default: - // We have not implemented a compression ... :( - err = fmt.Errorf( - "unsupported compression type %s - this is a bug in k6, please report it", - compression, - ) - } + decoder, err := pickDecoder(compression, rc) if err != nil { return nil, newDecompressionError(err) } + rc = &readCloser{decoder} } } @@ -209,3 +192,26 @@ func readResponseBody( return result, respErr } + +func pickDecoder(compression CompressionType, rc *readCloser) (io.Reader, error) { + var decoder io.Reader + var err error + switch compression { + case CompressionTypeDeflate: + decoder, err = zlib.NewReader(rc) + case CompressionTypeGzip: + decoder, err = gzip.NewReader(rc) + case CompressionTypeZstd: + decoder, err = zstd.NewReader(rc) + case CompressionTypeBr: + decoder = brotli.NewReader(rc) + default: + // We have not implemented a compression ... :( + err = fmt.Errorf( + "unsupported compression type %s - this is a bug in k6, please report it", + compression, + ) + } + + return decoder, err +} From 4970d55ffa73060df882733a407724dadeae96b7 Mon Sep 17 00:00:00 2001 From: Oleg Bespalov Date: Fri, 1 Dec 2023 09:59:17 +0100 Subject: [PATCH 5/5] Removes unused processRersponses callback --- js/modules/k6/http/request.go | 1 - lib/netext/httpext/batch.go | 3 --- 2 files changed, 4 deletions(-) diff --git a/js/modules/k6/http/request.go b/js/modules/k6/http/request.go index 832792571d56..87243e95403d 100644 --- a/js/modules/k6/http/request.go +++ b/js/modules/k6/http/request.go @@ -495,7 +495,6 @@ func (c *Client) Batch(reqsV ...goja.Value) (interface{}, error) { errs := httpext.MakeBatchRequests( c.moduleInstance.vu.Context(), state, batchReqs, reqCount, int(state.Options.Batch.Int64), int(state.Options.BatchPerHost.Int64), - c.processResponse, ) for i := 0; i < reqCount; i++ { diff --git a/lib/netext/httpext/batch.go b/lib/netext/httpext/batch.go index fd783336f975..60394e7ae13f 100644 --- a/lib/netext/httpext/batch.go +++ b/lib/netext/httpext/batch.go @@ -22,13 +22,10 @@ type BatchParsedHTTPRequest struct { // pre-initialized. In addition, each processed request would emit either a nil // value, or an error, via the returned errors channel. The goroutines exit when // the requests channel is closed. -// The processResponse callback can be used to modify the response, e.g. -// to replace the body. func MakeBatchRequests( ctx context.Context, state *lib.State, requests []BatchParsedHTTPRequest, reqCount, globalLimit, perHostLimit int, - processResponse func(*Response, ResponseType), ) <-chan error { workers := globalLimit if reqCount < workers {