Skip to content

Conversation

@metcoder95
Copy link
Member

@metcoder95 metcoder95 commented Mar 15, 2024

This relates to...

Rationale

Changes

Features

Bug Fixes

Breaking Changes and Deprecations

Status

@codecov-commenter
Copy link

codecov-commenter commented Mar 15, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 93.48%. Comparing base (3be7ec9) to head (8810a8a).

Additional details and impacted files
@@           Coverage Diff           @@
##             main    #2966   +/-   ##
=======================================
  Coverage   93.48%   93.48%           
=======================================
  Files          89       89           
  Lines       24171    24177    +6     
=======================================
+ Hits        22597    22603    +6     
  Misses       1574     1574           

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for opening a PR! Can you please add a unit test?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina
Copy link
Member

there is a conflict now, can you fix it?

: Math.min(currentTimeout * timeoutFactor ** counter, maxTimeout)

state.currentTimeout = retryTimeout
: Math.min(minTimeout * timeoutFactor ** counter, maxTimeout)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if counter starts at 1 should this not be (counter - 1)? This way the first retry uses the minTimeout value?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

right now it would start at double the original minTimeout

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm

@mcollina mcollina merged commit d4332fc into main Mar 21, 2024
@Uzlopak Uzlopak deleted the fix/retry-handler branch April 7, 2024 05:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants