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

Adressing linter issues in netext, netext/httpext packages #3484

Merged
merged 5 commits into from
Dec 12, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion js/modules/k6/http/request.go
Original file line number Diff line number Diff line change
Expand Up @@ -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++ {
Expand Down
2 changes: 1 addition & 1 deletion lib/netext/dialer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
3 changes: 0 additions & 3 deletions lib/netext/httpext/batch.go
Original file line number Diff line number Diff line change
Expand Up @@ -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),
olegbespalov marked this conversation as resolved.
Show resolved Hide resolved
) <-chan error {
workers := globalLimit
if reqCount < workers {
Expand Down
52 changes: 29 additions & 23 deletions lib/netext/httpext/compression.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"bytes"
"compress/gzip"
"compress/zlib"
"errors"
"fmt"
"io"
"net/http"
Expand Down Expand Up @@ -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)
}
}
Expand All @@ -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}
Expand All @@ -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}
}
}
Expand Down Expand Up @@ -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
}
2 changes: 1 addition & 1 deletion lib/netext/httpext/error_codes_syscall_posix.go
Original file line number Diff line number Diff line change
Expand Up @@ -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, ""
}
8 changes: 6 additions & 2 deletions lib/netext/httpext/error_codes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
})
Expand Down Expand Up @@ -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(),
),
Expand Down
5 changes: 4 additions & 1 deletion lib/netext/httpext/request_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
17 changes: 9 additions & 8 deletions lib/netext/httpext/tracer.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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()
}

Expand All @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -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())
}
Expand Down Expand Up @@ -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)
Expand Down
17 changes: 9 additions & 8 deletions lib/netext/httpext/tracer_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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")
}

Expand All @@ -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)
Expand All @@ -217,7 +218,7 @@ func TestTracerNegativeHttpSendingValues(t *testing.T) {
}

// make the next connection write fail
failOnConnWrite = true
connection.failOnConnWrite = true

{
tracer := &Tracer{}
Expand All @@ -244,7 +245,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(),
Expand Down
1 change: 1 addition & 0 deletions lib/netext/httpext/transport.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package httpext provides extensions to the standard net/http package
package httpext

import (
Expand Down
4 changes: 2 additions & 2 deletions lib/netext/httpext/transport_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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()
Expand Down
3 changes: 2 additions & 1 deletion lib/netext/resolver_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
7 changes: 6 additions & 1 deletion lib/netext/tls.go
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
// Package netext provides extensions to the standard net package
package netext

import (
Expand All @@ -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"
Expand All @@ -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"`
Expand All @@ -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 {
Expand Down