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

[grpc-js] Fix error messages in serviceConfig validation #2782

Conversation

matthewbinshtok
Copy link
Contributor

This PR corrects five error messages in grpc-js's serviceConfig validation:

  • 3 typos corrected from Invld to Invalid
  • 2 corrected to say that strings containing decimals are valid in initialBackoff and maxBackoff values of retryConfig.

Validation for initialBackoff and maxBackoff in retryConfig use the following regex which allows decimals:

/**
 * Recognizes a number with up to 9 digits after the decimal point, followed by
 * an "s", representing a number of seconds.
 */
const DURATION_REGEX = /^\d+(\.\d{1,9})?s$/;

However, the error messages for initialBackoff and maxBackoff in validateRetryPolicy state that the values "must be a string consisting of a positive integer followed by s".

I discovered this when correcting a copied Go serviceConfig containing decimals that did not start with a 0:

    ...
    "InitialBackoff": ".002s",
    "MaxBackoff": ".01s",

becomes

    ...
    "initialBackoff": "0.002s",
    "maxBackoff": "0.01s",

Copy link

linux-foundation-easycla bot commented Jun 26, 2024

CLA Signed

The committers listed above are authorized under a signed CLA.

  • ✅ login: matthewbinshtok / name: Matthew Binshtok (d6925d9, fc5bee7)

@matthewbinshtok
Copy link
Contributor Author

Test suites failed due to error message comparison, I've changed the tests to expect the new error message.

@matthewbinshtok
Copy link
Contributor Author

Latest test failures seem unrelated.

@murgatroid99 murgatroid99 merged commit d83355b into grpc:master Jun 27, 2024
3 of 5 checks passed
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.

3 participants