ronantakizawa/codereview-bench huggingface dataset:
Repository: traefik/traefik
File: pkg/middlewares/retry/retry.go (Go)

Code before:
	}
}

type responseWriter interface {
	http.ResponseWriter
	http.Flusher
	ShouldRetry() bool
	DisableRetries()
}

func newResponseWriter(rw http.ResponseWriter, shouldRetry bool) responseWriter {
	responseWriter := &responseWriterWithoutCloseNotify{
		responseWriter: rw,
		headers:        make(http.Header),
		shouldRetry:    shouldRetry,
	}
	if _, ok := rw.(http.CloseNotifier); ok {
		return &responseWriterWithCloseNotify{
			responseWriterWithoutCloseNotify: responseWriter,
		}
	}
	return responseWriter
}

type responseWriterWithoutCloseNotify struct {
	responseWriter http.ResponseWriter
	headers        http.Header
	shouldRetry    bool
	written        bool
}

func (r *responseWriterWithoutCloseNotify) ShouldRetry() bool {
	return r.shouldRetry
}

func (r *responseWriterWithoutCloseNotify) DisableRetries() {
	r.shouldRetry = false
}

func (r *responseWriterWithoutCloseNotify) Header() http.Header {
	if r.written {
		return r.responseWriter.Header()
	}
	return r.headers
}

func (r *responseWriterWithoutCloseNotify) Write(buf []byte) (int, error) {
	if r.ShouldRetry() {
		return len(buf), nil
	}
	return r.responseWriter.Write(buf)

Review comment (suggestion):
Thanks for your previous changes, i think the comment should clarify the purpose here, and not confuse with the max interval, what do you think ?
```suggestion
	// calculate the multiplier for the given number of attempts
	// so that applying the multiplier for the given number of attempts will not exceed 2 times the initial interval
	// it allows to control the progression along the attempts
```

Code after:
			}

			currentInterval = backOff.NextBackOff()

			attempts++

			log.FromContext(middlewares.GetLoggerCtx(req.Context(), r.name, typeName)).
				Debugf("New attempt %d for request: %v", attempts, req.URL)

			r.listener.Retried(req, attempts)

		case <-req.Context().Done():
			return
		}
	}
}

func (r *retry) newBackOff() nexter {
	if r.attempts < 2 || r.initialInterval <= 0 {
		return &backoff.ZeroBackOff{}
	}

	b := backoff.NewExponentialBackOff()
	b.InitialInterval = r.initialInterval

	// calculate the multiplier for the given number of attempts
	// so that applying the multiplier for the given number of attempts will not exceed 2 times the initial interval
	// it allows to control the progression along the attempts
	b.Multiplier = math.Pow(2, 1/float64(r.attempts-1))

	// according to docs, b.Reset() must be called before using
	b.Reset()
	return b
}

// Retried exists to implement the Listener interface. It calls Retried on each of its slice entries.
func (l Listeners) Retried(req *http.Request, attempt int) {
	for _, listener := range l {
		listener.Retried(req, attempt)
	}
}

type responseWriter interface {
	http.ResponseWriter
	http.Flusher
	ShouldRetry() bool
	DisableRetries()
}

func newResponseWriter(rw http.ResponseWriter, shouldRetry bool) responseWriter {
	responseWriter := &responseWriterWithoutCloseNotify{
Repository: traefik/traefik
File: pkg/provider/kubernetes/crd/kubernetes.go (Go)

Code before:

	data, err := json.Marshal(plugins)
	if err != nil {
		return nil, err
	}

	pc := map[string]dynamic.PluginConf{}
	err = json.Unmarshal(data, &pc)
	if err != nil {
		return nil, err
	}

	return pc, nil
}

func createCircuitBreakerMiddleware(circuitBreaker *v1alpha1.CircuitBreaker) (*dynamic.CircuitBreaker, error) {
	if circuitBreaker == nil {
		return nil, nil
	}

	cb := &dynamic.CircuitBreaker{Expression: circuitBreaker.Expression}
	cb.SetDefaults()

	if circuitBreaker.CheckPeriod != nil {
		err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String())
		if err != nil {
			return nil, err
		}
	}

	if circuitBreaker.FallbackDuration != nil {
		err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String())
		if err != nil {
			return nil, err
		}
	}

	if circuitBreaker.RecoveryDuration != nil {
		err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String())
		if err != nil {
			return nil, err
		}
	}

	return cb, nil
}

func createRateLimitMiddleware(rateLimit *v1alpha1.RateLimit) (*dynamic.RateLimit, error) {
	if rateLimit == nil {
		return nil, nil
	}

Review comment (suggestion):
```suggestion
		if err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String()); err != nil {
```

Code after:

	data, err := json.Marshal(plugins)
	if err != nil {
		return nil, err
	}

	pc := map[string]dynamic.PluginConf{}
	err = json.Unmarshal(data, &pc)
	if err != nil {
		return nil, err
	}

	return pc, nil
}

func createCircuitBreakerMiddleware(circuitBreaker *v1alpha1.CircuitBreaker) (*dynamic.CircuitBreaker, error) {
	if circuitBreaker == nil {
		return nil, nil
	}

	cb := &dynamic.CircuitBreaker{Expression: circuitBreaker.Expression}
	cb.SetDefaults()

	if circuitBreaker.CheckPeriod != nil {
		if err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String()); err != nil {
			return nil, err
		}
	}

	if circuitBreaker.FallbackDuration != nil {
		if err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String()); err != nil {
			return nil, err
		}
	}

	if circuitBreaker.RecoveryDuration != nil {
		if err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String()); err != nil {
			return nil, err
		}
	}

	return cb, nil
}

func createRateLimitMiddleware(rateLimit *v1alpha1.RateLimit) (*dynamic.RateLimit, error) {
	if rateLimit == nil {
		return nil, nil
	}

	rl := &dynamic.RateLimit{Average: rateLimit.Average}
	rl.SetDefaults()
Repository: traefik/traefik
File: pkg/provider/kubernetes/crd/kubernetes.go (Go)

Code before:
	err = json.Unmarshal(data, &pc)
	if err != nil {
		return nil, err
	}

	return pc, nil
}

func createCircuitBreakerMiddleware(circuitBreaker *v1alpha1.CircuitBreaker) (*dynamic.CircuitBreaker, error) {
	if circuitBreaker == nil {
		return nil, nil
	}

	cb := &dynamic.CircuitBreaker{Expression: circuitBreaker.Expression}
	cb.SetDefaults()

	if circuitBreaker.CheckPeriod != nil {
		err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String())
		if err != nil {
			return nil, err
		}
	}

	if circuitBreaker.FallbackDuration != nil {
		err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String())
		if err != nil {
			return nil, err
		}
	}

	if circuitBreaker.RecoveryDuration != nil {
		err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String())
		if err != nil {
			return nil, err
		}
	}

	return cb, nil
}

func createRateLimitMiddleware(rateLimit *v1alpha1.RateLimit) (*dynamic.RateLimit, error) {
	if rateLimit == nil {
		return nil, nil
	}

	rl := &dynamic.RateLimit{Average: rateLimit.Average}
	rl.SetDefaults()

	if rateLimit.Burst != nil {
		rl.Burst = *rateLimit.Burst
	}

Review comment (suggestion):
```suggestion
		if err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String()); err != nil {
```

Code after:
	err = json.Unmarshal(data, &pc)
	if err != nil {
		return nil, err
	}

	return pc, nil
}

func createCircuitBreakerMiddleware(circuitBreaker *v1alpha1.CircuitBreaker) (*dynamic.CircuitBreaker, error) {
	if circuitBreaker == nil {
		return nil, nil
	}

	cb := &dynamic.CircuitBreaker{Expression: circuitBreaker.Expression}
	cb.SetDefaults()

	if circuitBreaker.CheckPeriod != nil {
		if err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String()); err != nil {
			return nil, err
		}
	}

	if circuitBreaker.FallbackDuration != nil {
		if err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String()); err != nil {
			return nil, err
		}
	}

	if circuitBreaker.RecoveryDuration != nil {
		if err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String()); err != nil {
			return nil, err
		}
	}

	return cb, nil
}

func createRateLimitMiddleware(rateLimit *v1alpha1.RateLimit) (*dynamic.RateLimit, error) {
	if rateLimit == nil {
		return nil, nil
	}

	rl := &dynamic.RateLimit{Average: rateLimit.Average}
	rl.SetDefaults()

	if rateLimit.Burst != nil {
		rl.Burst = *rateLimit.Burst
	}

	if rateLimit.Period != nil {
		err := rl.Period.Set(rateLimit.Period.String())
Repository: traefik/traefik
File: pkg/provider/kubernetes/crd/kubernetes.go (Go)

Code before:

func createCircuitBreakerMiddleware(circuitBreaker *v1alpha1.CircuitBreaker) (*dynamic.CircuitBreaker, error) {
	if circuitBreaker == nil {
		return nil, nil
	}

	cb := &dynamic.CircuitBreaker{Expression: circuitBreaker.Expression}
	cb.SetDefaults()

	if circuitBreaker.CheckPeriod != nil {
		err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String())
		if err != nil {
			return nil, err
		}
	}

	if circuitBreaker.FallbackDuration != nil {
		err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String())
		if err != nil {
			return nil, err
		}
	}

	if circuitBreaker.RecoveryDuration != nil {
		err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String())
		if err != nil {
			return nil, err
		}
	}

	return cb, nil
}

func createRateLimitMiddleware(rateLimit *v1alpha1.RateLimit) (*dynamic.RateLimit, error) {
	if rateLimit == nil {
		return nil, nil
	}

	rl := &dynamic.RateLimit{Average: rateLimit.Average}
	rl.SetDefaults()

	if rateLimit.Burst != nil {
		rl.Burst = *rateLimit.Burst
	}

	if rateLimit.Period != nil {
		err := rl.Period.Set(rateLimit.Period.String())
		if err != nil {
			return nil, err
		}
	}

Review comment (suggestion):
```suggestion
		if err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String()); err != nil {
```

Code after:

func createCircuitBreakerMiddleware(circuitBreaker *v1alpha1.CircuitBreaker) (*dynamic.CircuitBreaker, error) {
	if circuitBreaker == nil {
		return nil, nil
	}

	cb := &dynamic.CircuitBreaker{Expression: circuitBreaker.Expression}
	cb.SetDefaults()

	if circuitBreaker.CheckPeriod != nil {
		if err := cb.CheckPeriod.Set(circuitBreaker.CheckPeriod.String()); err != nil {
			return nil, err
		}
	}

	if circuitBreaker.FallbackDuration != nil {
		if err := cb.FallbackDuration.Set(circuitBreaker.FallbackDuration.String()); err != nil {
			return nil, err
		}
	}

	if circuitBreaker.RecoveryDuration != nil {
		if err := cb.RecoveryDuration.Set(circuitBreaker.RecoveryDuration.String()); err != nil {
			return nil, err
		}
	}

	return cb, nil
}

func createRateLimitMiddleware(rateLimit *v1alpha1.RateLimit) (*dynamic.RateLimit, error) {
	if rateLimit == nil {
		return nil, nil
	}

	rl := &dynamic.RateLimit{Average: rateLimit.Average}
	rl.SetDefaults()

	if rateLimit.Burst != nil {
		rl.Burst = *rateLimit.Burst
	}

	if rateLimit.Period != nil {
		err := rl.Period.Set(rateLimit.Period.String())
		if err != nil {
			return nil, err
		}
	}

	if rateLimit.SourceCriterion != nil {
		rl.SourceCriterion = rateLimit.SourceCriterion
Repository: traefik/traefik
File: pkg/muxer/tcp/mux.go (Go)

Code before:
func clientIP(tree *matchersTree, clientIPs ...string) error {
	checker, err := ip.NewChecker(clientIPs)
	if err != nil {
		return fmt.Errorf("could not initialize IP Checker for \"ClientIP\" matcher: %w", err)
	}

	tree.matcher = func(meta ConnData) bool {
		if meta.remoteIP == "" {
			return false
		}

		ok, err := checker.Contains(meta.remoteIP)
		if err != nil {
			log.WithoutContext().Warnf("\"ClientIP\" matcher: could not match remote address: %v", err)
			return false
		}
		return ok
	}

	return nil
}

// alpn checks if any of the connection ALPN protocols matches one of the matcher protocols.
func alpn(tree *matchersTree, protos ...string) error {
	if len(protos) == 0 {
		return fmt.Errorf("empty value for \"ALPN\" matcher is not allowed")
	}

	for _, proto := range protos {
		if proto == tlsalpn01.ACMETLS1Protocol {
			return fmt.Errorf("invalid protocol value for \"ALPN\" matcher, %q is not allowed", proto)
		}
	}

	tree.matcher = func(meta ConnData) bool {
		for _, proto := range meta.alpnProtos {
			for _, filter := range protos {
				if proto == filter {
					return true
				}
			}
		}

		return false
	}

	return nil
}

var almostFQDN = regexp.MustCompile(`^[[:alnum:]\.-]+$`)


Review comment (suggestion):
```suggestion
		return errors.New("empty value for \"ALPN\" matcher is not allowed")
```

Code after:
func clientIP(tree *matchersTree, clientIPs ...string) error {
	checker, err := ip.NewChecker(clientIPs)
	if err != nil {
		return fmt.Errorf("could not initialize IP Checker for \"ClientIP\" matcher: %w", err)
	}

	tree.matcher = func(meta ConnData) bool {
		if meta.remoteIP == "" {
			return false
		}

		ok, err := checker.Contains(meta.remoteIP)
		if err != nil {
			log.WithoutContext().Warnf("\"ClientIP\" matcher: could not match remote address: %v", err)
			return false
		}
		return ok
	}

	return nil
}

// alpn checks if any of the connection ALPN protocols matches one of the matcher protocols.
func alpn(tree *matchersTree, protos ...string) error {
	if len(protos) == 0 {
		return errors.New("empty value for \"ALPN\" matcher is not allowed")
	}

	for _, proto := range protos {
		if proto == tlsalpn01.ACMETLS1Protocol {
			return fmt.Errorf("invalid protocol value for \"ALPN\" matcher, %q is not allowed", proto)
		}
	}

	tree.matcher = func(meta ConnData) bool {
		for _, proto := range meta.alpnProtos {
			for _, filter := range protos {
				if proto == filter {
					return true
				}
			}
		}

		return false
	}

	return nil
}

var almostFQDN = regexp.MustCompile(`^[[:alnum:]\.-]+$`)
Repository: traefik/traefik
File: pkg/provider/ecs/ecs.go (Go)

Code before:
		input := &ecs.ListTasksInput{
			Cluster:       &c,
			DesiredStatus: aws.String(ecs.DesiredStatusRunning),
		}

		tasks := make(map[string]*ecs.Task)
		err := client.ecs.ListTasksPagesWithContext(ctx, input, func(page *ecs.ListTasksOutput, lastPage bool) bool {
			if len(page.TaskArns) > 0 {
				resp, err := client.ecs.DescribeTasksWithContext(ctx, &ecs.DescribeTasksInput{
					Tasks:   page.TaskArns,
					Cluster: &c,
				})
				if err != nil {
					logger.Errorf("Unable to describe tasks for %v", page.TaskArns)
				} else {
					for _, t := range resp.Tasks {
						if aws.StringValue(t.LastStatus) == ecs.DesiredStatusRunning {
							tasks[aws.StringValue(t.TaskArn)] = t
						}
					}
				}
			}
			return !lastPage
		})
		if err != nil {
			return nil, fmt.Errorf("list tasks: %w", err)
		}

		// Skip to the next cluster if there are no tasks found on
		// this cluster.
		if len(tasks) == 0 {
			continue
		}

		ec2Instances, err := p.lookupEc2Instances(ctx, client, &c, tasks)
		if err != nil {
			return nil, err
		}

		taskDefinitions, err := p.lookupTaskDefinitions(ctx, client, tasks)
		if err != nil {
			return nil, err
		}

		for key, task := range tasks {
			containerInstance := ec2Instances[aws.StringValue(task.ContainerInstanceArn)]
			taskDef := taskDefinitions[key]

			for _, container := range task.Containers {
				var containerDefinition *ecs.ContainerDefinition
				for _, def := range taskDef.ContainerDefinitions {

Review comment (suggestion):
```suggestion
			return nil, fmt.Errorf("listing tasks: %w", err)
```

Code after:
		input := &ecs.ListTasksInput{
			Cluster:       &c,
			DesiredStatus: aws.String(ecs.DesiredStatusRunning),
		}

		tasks := make(map[string]*ecs.Task)
		err := client.ecs.ListTasksPagesWithContext(ctx, input, func(page *ecs.ListTasksOutput, lastPage bool) bool {
			if len(page.TaskArns) > 0 {
				resp, err := client.ecs.DescribeTasksWithContext(ctx, &ecs.DescribeTasksInput{
					Tasks:   page.TaskArns,
					Cluster: &c,
				})
				if err != nil {
					logger.Errorf("Unable to describe tasks for %v", page.TaskArns)
				} else {
					for _, t := range resp.Tasks {
						if aws.StringValue(t.LastStatus) == ecs.DesiredStatusRunning {
							tasks[aws.StringValue(t.TaskArn)] = t
						}
					}
				}
			}
			return !lastPage
		})
		if err != nil {
			return nil, fmt.Errorf("listing tasks: %w", err)
		}

		// Skip to the next cluster if there are no tasks found on
		// this cluster.
		if len(tasks) == 0 {
			continue
		}

		ec2Instances, err := p.lookupEc2Instances(ctx, client, &c, tasks)
		if err != nil {
			return nil, err
		}

		taskDefinitions, err := p.lookupTaskDefinitions(ctx, client, tasks)
		if err != nil {
			return nil, err
		}

		for key, task := range tasks {
			containerInstance := ec2Instances[aws.StringValue(task.ContainerInstanceArn)]
			taskDef := taskDefinitions[key]

			for _, container := range task.Containers {
				var containerDefinition *ecs.ContainerDefinition
				for _, def := range taskDef.ContainerDefinitions {
Repository: traefik/traefik
File: pkg/provider/ecs/ecs.go (Go)

Code before:
				}
				return !lastPage
			})
			if err != nil {
				return nil, fmt.Errorf("describe instances: %w", err)
			}
		}
	}

	return ec2Instances, nil
}

func (p *Provider) lookupTaskDefinitions(ctx context.Context, client *awsClient, taskDefArns map[string]*ecs.Task) (map[string]*ecs.TaskDefinition, error) {
	logger := log.FromContext(ctx)
	taskDef := make(map[string]*ecs.TaskDefinition)

	for arn, task := range taskDefArns {
		if definition, ok := existingTaskDefCache.Get(arn); ok {
			taskDef[arn] = definition.(*ecs.TaskDefinition)
			logger.Debugf("Found cached task definition for %s. Skipping the call", arn)
		} else {
			resp, err := client.ecs.DescribeTaskDefinitionWithContext(ctx, &ecs.DescribeTaskDefinitionInput{
				TaskDefinition: task.TaskDefinitionArn,
			})
			if err != nil {
				return nil, fmt.Errorf("describe task definition: %w", err)
			}

			taskDef[arn] = resp.TaskDefinition
			existingTaskDefCache.Set(arn, resp.TaskDefinition, cache.DefaultExpiration)
		}
	}
	return taskDef, nil
}

// chunkIDs ECS expects no more than 100 parameters be passed to a API call;
// thus, pack each string into an array capped at 100 elements.
func (p *Provider) chunkIDs(ids []*string) [][]*string {
	var chunked [][]*string
	for i := 0; i < len(ids); i += 100 {
		var sliceEnd int
		if i+100 < len(ids) {
			sliceEnd = i + 100
		} else {
			sliceEnd = len(ids)
		}
		chunked = append(chunked, ids[i:sliceEnd])
	}
	return chunked
}


Review comment (suggestion):
```suggestion
				return nil, fmt.Errorf("describing task definition: %w", err)
```

Code after:
				}
				return !lastPage
			})
			if err != nil {
				return nil, fmt.Errorf("describing instances: %w", err)
			}
		}
	}

	return ec2Instances, nil
}

func (p *Provider) lookupTaskDefinitions(ctx context.Context, client *awsClient, taskDefArns map[string]*ecs.Task) (map[string]*ecs.TaskDefinition, error) {
	logger := log.FromContext(ctx)
	taskDef := make(map[string]*ecs.TaskDefinition)

	for arn, task := range taskDefArns {
		if definition, ok := existingTaskDefCache.Get(arn); ok {
			taskDef[arn] = definition.(*ecs.TaskDefinition)
			logger.Debugf("Found cached task definition for %s. Skipping the call", arn)
		} else {
			resp, err := client.ecs.DescribeTaskDefinitionWithContext(ctx, &ecs.DescribeTaskDefinitionInput{
				TaskDefinition: task.TaskDefinitionArn,
			})
			if err != nil {
				return nil, fmt.Errorf("describing task definition: %w", err)
			}

			taskDef[arn] = resp.TaskDefinition
			existingTaskDefCache.Set(arn, resp.TaskDefinition, cache.DefaultExpiration)
		}
	}
	return taskDef, nil
}

// chunkIDs ECS expects no more than 100 parameters be passed to a API call;
// thus, pack each string into an array capped at 100 elements.
func (p *Provider) chunkIDs(ids []*string) [][]*string {
	var chunked [][]*string
	for i := 0; i < len(ids); i += 100 {
		var sliceEnd int
		if i+100 < len(ids) {
			sliceEnd = i + 100
		} else {
			sliceEnd = len(ids)
		}
		chunked = append(chunked, ids[i:sliceEnd])
	}
	return chunked
}
Repository: traefik/traefik
File: pkg/tcp/proxy_unix.go (Go)

Code before:
//go:build !windows
// +build !windows

package tcp

import (
	"errors"
	"net"
	"syscall"
)

// isReadConnResetError reports whether err is a connection reset error during a read operation.
func isReadConnResetError(err error) bool {
	var oerr *net.OpError
	if errors.As(err, &oerr) && oerr.Op == "read" {
		return errors.Is(err, syscall.ECONNRESET)
	}

	return false
}


Review comment (suggestion):
```suggestion
	return errors.As(err, &oerr) && oerr.Op == "read" && errors.Is(err, syscall.ECONNRESET)
```

Code after:
//go:build !windows
// +build !windows

package tcp

import (
	"errors"
	"net"
	"syscall"
)

// isReadConnResetError reports whether err is a connection reset error during a read operation.
func isReadConnResetError(err error) bool {
	var oerr *net.OpError
	return errors.As(err, &oerr) && oerr.Op == "read" && errors.Is(err, syscall.ECONNRESET)
}
Repository: traefik/traefik
File: pkg/tcp/proxy_windows.go (Go)

Code before:
//go:build windows
// +build windows

package tcp

import (
	"errors"
	"net"
	"syscall"
)

// isReadConnResetError reports whether err is a connection reset error during a read operation.
func isReadConnResetError(err error) bool {
	var oerr *net.OpError
	if errors.As(err, &oerr) && oerr.Op == "read" {
		return errors.Is(err, syscall.WSAECONNRESET)
	}

	return false
}


Review comment (suggestion):
```suggestion
	return errors.As(err, &oerr) && oerr.Op == "read" && errors.Is(err, syscall.WSAECONNRESET)
```

Code after:
//go:build windows
// +build windows

package tcp

import (
	"errors"
	"net"
	"syscall"
)

// isReadConnResetError reports whether err is a connection reset error during a read operation.
func isReadConnResetError(err error) bool {
	var oerr *net.OpError
	return errors.As(err, &oerr) && oerr.Op == "read" && errors.Is(err, syscall.WSAECONNRESET)
}