Skip to content

FFM-11972 Add authRequestReadTimeout option#135

Merged
erdirowlands merged 6 commits intomainfrom
FFM-11972
Sep 3, 2024
Merged

FFM-11972 Add authRequestReadTimeout option#135
erdirowlands merged 6 commits intomainfrom
FFM-11972

Conversation

@erdirowlands
Copy link
Contributor

@erdirowlands erdirowlands commented Sep 2, 2024

What

Adds configuration option authRequestReadTimeout which allows users to specify a timeout for the authentication request made by the SDK. The default value is 0 (to keep backwards compatability) which means no timeout will occur. If set to a value greater than 0, the request will abort. An error will be logged and the ERROR_AUTH and ERROR events will be emitted.
For older browsers (prior to 2016) which do not support the AbortController; if authRequestReadTimeout is configured the auth request will proceed without a timeout and a warning will be logged.

Also patches GHSA-952p-6rrq-rcjv. This is found in micromatch which is used by dev dependencies and not shipped to users.

  • @types/jest@29.5.4
  • "jest": "^29.6.4",

Testing

Using a local proxy tool to simulate network delays:

  • Default (authRequestReadTimeout not supplied meaning no timeout):

    • With delay: The request hangs indefinitely and does not timeout, as expected.
    • Without delay: The authentication completes correctly without any issues.
  • Configured timeout:

    • With delay: The request is correctly aborted after the specified timeout, and an appropriate error is logged (Request timed out).
    • Without delay: The authentication completes correctly within the specified timeout.

);
```

## API Middleware
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Deciding to document this here, mainly to show users that they can use this to timeout the remaining SDK requests like evaluations / metrics

src/index.ts Outdated
try {
const response = await fetch(url, requestOptions)

if (!response.ok) {
Copy link
Contributor Author

Choose a reason for hiding this comment

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

We never checked the response was successful before. We relied on flakey behaviour of parsing the JWT token which would result in an invalid token exception logged as the reason. We now get to see the actual http error.

Copy link
Contributor

@davejohnston davejohnston left a comment

Choose a reason for hiding this comment

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

This looks good.

@erdirowlands erdirowlands merged commit 8ea98bb into main Sep 3, 2024
@erdirowlands erdirowlands deleted the FFM-11972 branch September 3, 2024 11:25
github-merge-queue bot referenced this pull request in open-feature/playground Sep 12, 2024
…1.28.0 (#372)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
|
[@harnessio/ff-javascript-client-sdk](https://redirect.github.com/harness/ff-javascript-client-sdk)
| [`1.26.2` ->
`1.28.0`](https://renovatebot.com/diffs/npm/@harnessio%2fff-javascript-client-sdk/1.26.2/1.28.0)
|
[![age](https://developer.mend.io/api/mc/badges/age/npm/@harnessio%2fff-javascript-client-sdk/1.28.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/@harnessio%2fff-javascript-client-sdk/1.28.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/@harnessio%2fff-javascript-client-sdk/1.26.2/1.28.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/@harnessio%2fff-javascript-client-sdk/1.26.2/1.28.0?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>harness/ff-javascript-client-sdk
(@&#8203;harnessio/ff-javascript-client-sdk)</summary>

###
[`v1.28.0`](https://redirect.github.com/harness/ff-javascript-client-sdk/releases/tag/1.28.0)

[Compare
Source](https://redirect.github.com/harness/ff-javascript-client-sdk/compare/1.27.0...1.28.0)

#### What's Changed

Enhancements:
FFM-11972 Add`authRequestReadTimeout` config option - see
[readme](https://redirect.github.com/harness/ff-javascript-client-sdk/blob/main/README.md#authentication-request-timeout)
for further information and how to enable
[@&#8203;erdirowlands](https://redirect.github.com/erdirowlands) in
[https://github.com/harness/ff-javascript-client-sdk/pull/135](https://redirect.github.com/harness/ff-javascript-client-sdk/pull/135)

Bug fixes
FFM-11972 If authentication fails the the correct error will be logged
instead of `Invalid Token` by
[@&#8203;erdirowlands](https://redirect.github.com/erdirowlands) in
[https://github.com/harness/ff-javascript-client-sdk/pull/135](https://redirect.github.com/harness/ff-javascript-client-sdk/pull/135)

**Full Changelog**:
harness/ff-javascript-client-sdk@1.27.0...1.28.0

###
[`v1.27.0`](https://redirect.github.com/harness/ff-javascript-client-sdk/releases/tag/1.27.0)

[Compare
Source](https://redirect.github.com/harness/ff-javascript-client-sdk/compare/1.26.3...1.27.0)

#### What's Changed

##### Enhancements:

- FFM-11788 Add `maxStreamRetries` config option by
[@&#8203;erdirowlands](https://redirect.github.com/erdirowlands) in
[https://github.com/harness/ff-javascript-client-sdk/pull/126](https://redirect.github.com/harness/ff-javascript-client-sdk/pull/126)

If retries are exhausted, one of two states can occur:

1. If Polling is enabled, the SDK will remain in polling mode and no
further streaming reconnection attempts will be made. The default
polling option, if not supplied, is whatever the streamingEnabled value
is.
    or
2. If polling is disabled, the SDK will not get any further evaluation
updates for the remainder of the SDK client instance's life. The SDK
will need re-initialised, e.g the app being restarted, to get new
evaluations in this state.

##### Bug fixes

FFM-11852 Fixes an edge case where if the stream disconnects and resumes
during a request made by the fallback poller (60 seconds later), the
fallback poller will not be disabled and will continue polling for the
lifetime of the SDK instance by
[@&#8203;erdirowlands](https://redirect.github.com/erdirowlands) in
[https://github.com/harness/ff-javascript-client-sdk/pull/131](https://redirect.github.com/harness/ff-javascript-client-sdk/pull/131)

**Full Changelog**:
harness/ff-javascript-client-sdk@1.26.3...1.27.0

###
[`v1.26.3`](https://redirect.github.com/harness/ff-javascript-client-sdk/releases/tag/1.26.3)

[Compare
Source](https://redirect.github.com/harness/ff-javascript-client-sdk/compare/1.26.2...1.26.3)

#### What's Changed

-   Maintenance: Patches the following CVEs
    -   GHSA-3h5v-q93c-6h6q
    -   GHSA-grv7-fg5c-xmjg

[https://github.com/harness/ff-javascript-client-sdk/pull/127](https://redirect.github.com/harness/ff-javascript-client-sdk/pull/127)pull/127
by [@&#8203;erdirowlands](https://redirect.github.com/erdirowlands)

**Full Changelog**:
harness/ff-javascript-client-sdk@1.26.2...1.26.3

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Enabled.

♻ **Rebasing**: Whenever PR is behind base branch, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR was generated by [Mend Renovate](https://mend.io/renovate/).
View the [repository job
log](https://developer.mend.io/github/open-feature/playground).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzOC43NC4xIiwidXBkYXRlZEluVmVyIjoiMzguNzQuMSIsInRhcmdldEJyYW5jaCI6Im1haW4iLCJsYWJlbHMiOlsicmVub3ZhdGUiXX0=-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
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.

2 participants