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

Option mismatch in RetryHandler #2961

Closed
acommodari opened this issue Mar 14, 2024 · 2 comments
Closed

Option mismatch in RetryHandler #2961

acommodari opened this issue Mar 14, 2024 · 2 comments
Labels
bug Something isn't working

Comments

@acommodari
Copy link
Contributor

Bug Description

There seems to be an option mismatch in the RetryHandler.

Reproducible By

const client = new Client("http://localhost:3000").compose(
  retry({
    retry: function (err, context, callback) {
      console.log("retrying", {
        err: err.code,
        state: context.state,
        opts: context.opts.retryOptions,
      });
      if (context.state.counter >= context.opts.retryOptions.maxRetries) {
        callback(err);
        return;
      }
      // Both the docs and types made me thing that context.opts.retryOptions.minTimeout 
      // should exist as an option but it in reality is context.opts.retryOptions.timeout
      setTimeout(() => callback(null), context.opts.retryOptions.timeout);
    },
  }),
  minTimeout: 400, // here it is called minTimeout
);

Expected Behavior

Either the property in the code should be changed from timeout to minTimeout or the docs + types should be updated to reflect the difference.

Logs & Screenshots

in retry-handler.js of undici v6.9.0 on line 40

class RetryHandler {
  constructor (opts, handlers) {
    const { retryOptions, ...dispatchOpts } = opts
    const {
      // Retry scoped
      retry: retryFn,
      maxRetries,
      maxTimeout,
      minTimeout,
      timeoutFactor,
      // Response scoped
      methods,
      errorCodes,
      retryAfter,
      statusCodes
    } = retryOptions ?? {}

    this.dispatch = handlers.dispatch
    this.handler = handlers.handler
    this.opts = dispatchOpts
    this.abort = null
    this.aborted = false
    this.retryOpts = {
      retry: retryFn ?? RetryHandler[kRetryHandlerDefaultRetry],
      retryAfter: retryAfter ?? true,
      maxTimeout: maxTimeout ?? 30 * 1000, // 30s,
      timeout: minTimeout ?? 500, // .5s. // <=== here we are setting the value to timeout
      timeoutFactor: timeoutFactor ?? 2,
      maxRetries: maxRetries ?? 5,

...

Environment

macOS Ventura 13.2.1
node 20.11.1
npm 10.2.4

Additional context

I am willing to open a PR myself if needed.

@acommodari acommodari added the bug Something isn't working label Mar 14, 2024
@metcoder95
Copy link
Member

Thanks for the report! A PR is always welcomed 👍

@acommodari
Copy link
Contributor Author

PR opened 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants