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

fix: improve handling of streaming error state changes/logging #439

Merged
merged 1 commit into from
Sep 30, 2024

Conversation

cwaldren-ld
Copy link
Contributor

@cwaldren-ld cwaldren-ld commented Sep 27, 2024

The existing handling for data source state transitions when receiving sse errors was incorrect. It was treating any error as a state transition to kOff/kShutdown, and logging at the error level.

The correct behavior would be making that transition only if the error is unrecoverable. Additionally, recoverable errors should be logged at the debug level, in order to not overstate the impact of the issue.

This bug did not actually affect the backoff behavior of the eventsource, but it did impact any status listeners which was certainly misleading.

@cwaldren-ld cwaldren-ld marked this pull request as ready for review September 28, 2024 00:10
@cwaldren-ld cwaldren-ld requested a review from a team as a code owner September 28, 2024 00:10
@cwaldren-ld cwaldren-ld added package: sdk/client Issues affecting the C++ Client SDK package: shared/sse Issues affecting the C++ Server Sent Events Library package: sdk/server Issues affecting the C++ Server SDK labels Sep 28, 2024
@cwaldren-ld cwaldren-ld merged commit 04e7e0e into main Sep 30, 2024
24 checks passed
@cwaldren-ld cwaldren-ld deleted the cw/sdk-727/unrecoverable-error branch September 30, 2024 19:48
@github-actions github-actions bot mentioned this pull request Sep 30, 2024
cwaldren-ld added a commit that referenced this pull request Sep 30, 2024
The existing handling for data source state transitions when receiving
`sse` errors was incorrect. It was treating any error as a state
transition to `kOff/kShutdown`, and logging at the `error` level.

The correct behavior would be making that transition only if the error
is unrecoverable. Additionally, recoverable errors should be logged at
the debug level, in order to not overstate the impact of the issue.

This bug did not actually affect the backoff behavior of the
eventsource, but it did impact status listeners and any code that queried the existing status within the SDK.
cwaldren-ld added a commit that referenced this pull request Sep 30, 2024
The existing handling for data source state transitions when receiving
`sse` errors was incorrect. It was treating any error as a state
transition to `kOff/kShutdown`, and logging at the `error` level.

The correct behavior would be making that transition only if the error
is unrecoverable. Additionally, recoverable errors should be logged at
the debug level, in order to not overstate the impact of the issue.

This bug did not actually affect the backoff behavior of the
eventsource, but it did impact status listeners and any code that queried the existing status within the SDK.
cwaldren-ld added a commit that referenced this pull request Sep 30, 2024
The existing handling for data source state transitions when receiving
`sse` errors was incorrect. It was treating any error as a state
transition to `kOff/kShutdown`, and logging at the `error` level.

The correct behavior would be making that transition only if the error
is unrecoverable. Additionally, recoverable errors should be logged at
the debug level, in order to not overstate the impact of the issue.

This bug did not actually affect the backoff behavior of the
eventsource, but it did impact status listeners and any code that queried the existing status within the SDK.
cwaldren-ld added a commit that referenced this pull request Sep 30, 2024
The existing handling for data source state transitions when receiving
`sse` errors was incorrect. It was treating any error as a state
transition to `kOff/kShutdown`, and logging at the `error` level.

The correct behavior would be making that transition only if the error
is unrecoverable. Additionally, recoverable errors should be logged at
the debug level, in order to not overstate the impact of the issue.

This bug did not actually affect the backoff behavior of the
eventsource, but it did impact status listeners and any code that queried the existing status within the SDK.
cwaldren-ld pushed a commit that referenced this pull request Oct 1, 2024
🤖 I have created a release *beep* *boop*
---


<details><summary>launchdarkly-cpp-client: 3.7.0</summary>

##
[3.7.0](launchdarkly-cpp-client-v3.6.4...launchdarkly-cpp-client-v3.7.0)
(2024-10-01)


### Features

* add client-side C binding for fetching data source state
([#442](#442))
([e8ca616](e8ca616))


### Bug Fixes

* client init state should not regress when data source interrupted
([#441](#441))
([4a7f1f9](4a7f1f9))
* improve handling of streaming error state changes/logging
([#439](#439))
([04e7e0e](04e7e0e))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-internal bumped from 0.8.2 to 0.8.3
    * launchdarkly-cpp-sse-client bumped from 0.5.2 to 0.5.3
</details>

<details><summary>launchdarkly-cpp-internal: 0.8.3</summary>

##
[0.8.3](launchdarkly-cpp-internal-v0.8.2...launchdarkly-cpp-internal-v0.8.3)
(2024-10-01)


### Bug Fixes

* improve handling of streaming error state changes/logging
([#439](#439))
([04e7e0e](04e7e0e))
</details>

<details><summary>launchdarkly-cpp-server: 3.6.1</summary>

##
[3.6.1](launchdarkly-cpp-server-v3.6.0...launchdarkly-cpp-server-v3.6.1)
(2024-10-01)


### Bug Fixes

* improve caching behavior of database integration
([#444](#444))
([5f47864](5f47864))
* improve handling of streaming error state changes/logging
([#439](#439))
([04e7e0e](04e7e0e))


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-internal bumped from 0.8.2 to 0.8.3
    * launchdarkly-cpp-sse-client bumped from 0.5.2 to 0.5.3
</details>

<details><summary>launchdarkly-cpp-server-redis-source: 2.1.13</summary>

##
[2.1.13](launchdarkly-cpp-server-redis-source-v2.1.12...launchdarkly-cpp-server-redis-source-v2.1.13)
(2024-10-01)


### Dependencies

* The following workspace dependencies were updated
  * dependencies
    * launchdarkly-cpp-server bumped from 3.6.0 to 3.6.1
</details>

<details><summary>launchdarkly-cpp-sse-client: 0.5.3</summary>

##
[0.5.3](launchdarkly-cpp-sse-client-v0.5.2...launchdarkly-cpp-sse-client-v0.5.3)
(2024-10-01)


### Bug Fixes

* improve handling of streaming error state changes/logging
([#439](#439))
([04e7e0e](04e7e0e))
</details>

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: sdk/client Issues affecting the C++ Client SDK package: sdk/server Issues affecting the C++ Server SDK package: shared/sse Issues affecting the C++ Server Sent Events Library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants