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.

Sign up for GitHub

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

Jump to bottom

fix: run error hook when provider returns reason error or error code #926

Merged
merged 6 commits into from
May 8, 2024
Merged

fix: run error hook when provider returns reason error or error code #926

merged 6 commits into from
May 8, 2024

Conversation

beeme1mr
Copy link
Member

@beeme1mr beeme1mr commented May 1, 2024

This PR

  • runs error hook when provider returns reason error or error code

Related Issues

Fixes #925

Notes

Based on a conversation in Slack: https://cloud-native.slack.com/archives/C06E4DE6S07/p1714581197391509

beeme1mr added 2 commits May 1, 2024 14:30
@beeme1mr
fix react test config
50a39e0 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
runs error hook when provide returns error code
5bc81b9 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr requested a review from a team as a code owner May 1, 2024 18:32
@beeme1mr beeme1mr linked an issue May 1, 2024 that may be closed by this pull request
[BUG] Error hooks only run if the provider throws #925
Closed
beeme1mr
beeme1mr commented May 1, 2024
View reviewed changes
packages/client/src/client/open-feature-client.ts Outdated Show resolved Hide resolved
packages/client/test/hooks.spec.ts Outdated Show resolved Hide resolved
lukas-reining
lukas-reining approved these changes May 2, 2024
View reviewed changes
Copy link
Member

@lukas-reining lukas-reining left a comment

Choose a reason for hiding this comment

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

Nice, I like the change!

packages/client/src/client/open-feature-client.ts Outdated Show resolved Hide resolved
beeme1mr added 2 commits May 2, 2024 10:59
@beeme1mr
Update packages/shared/src/errors/index.ts
3962b9d 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
rename method name
6c24b96 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@toddbaert
Copy link
Member

toddbaert commented May 6, 2024 •
edited
Loading

Adding @maxveldink @federicobond @Kavindu-Dodan for Ruby/Python/Go SDK reference. I think we should add something to the spec about this, yes.

@beeme1mr
remove reason error as a condition
529abcc 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr
throw when eval details contain error code
d81ea32 
Signed-off-by: Michael Beemer <beeme1mr@users.noreply.github.com>
@beeme1mr beeme1mr added this pull request to the merge queue May 8, 2024
Merged via the queue into main with commit c6d0b5d May 8, 2024
9 checks passed
@beeme1mr beeme1mr deleted the 925-bug-error-hooks-only-run-if-the-provider-throws branch May 8, 2024 17:39
This was referenced May 7, 2024
Merged
Merged
Merged
@Kavindu-Dodan
Copy link

@toddbaert @beeme1mr

In Go, there's no concept of throwing, so we rely on error details 1 already

I created a follow-up for Java SDK 2

Footnotes

  1. ↩
  2. ↩

github-merge-queue bot pushed a commit that referenced this pull request May 13, 2024
@openfeaturebot @toddbaert
chore(main): release core 1.2.0 (#927)
692ad5b 
🤖 I have created a release *beep* *boop*
---


##
[1.2.0](core-v1.1.0...core-v1.2.0)
(2024-05-13)


### ✨ New Features

* set context during provider init on web
([#919](#919))
([7e6c1c6](7e6c1c6))


### 🐛 Bug Fixes

* removes exports of OpenFeatureClient class and makes event props
readonly ([#918](#918))
([e9a25c2](e9a25c2))
* run error hook when provider returns reason error or error code
([#926](#926))
([c6d0b5d](c6d0b5d))

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

---------

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Signed-off-by: openfeature-peer-update-bot <openfeature-peer-update-bot@openfeature.dev>
Co-authored-by: openfeature-peer-update-bot <openfeature-peer-update-bot@openfeature.dev>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert added a commit that referenced this pull request May 13, 2024
@openfeaturebot @toddbaert
chore(main): release server-sdk 1.14.0 (#930)
f2ebd11 
🤖 I have created a release *beep* *boop*
---


##
[1.14.0](server-sdk-v1.13.5...server-sdk-v1.14.0)
(2024-05-13)


### 🐛 Bug Fixes

* remove export of OpenFeatureClient
([#794](#794))
([3d197f2](3d197f2))
* removes exports of OpenFeatureClient class and makes event props
readonly ([#918](#918))
([e9a25c2](e9a25c2))
* run error hook when provider returns reason error or error code
([#926](#926))
([c6d0b5d](c6d0b5d))


### 🧹 Chore

* remove node 16
([#875](#875))
([c1878e4](c1878e4))
* **main:** release core 1.2.0
([#927](#927))
([692ad5b](692ad5b))


### 📚 Documentation

* add tip about supported usage in the install section
([#941](#941))
([f0de667](f0de667))

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

---------

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
toddbaert added a commit that referenced this pull request May 13, 2024
@openfeaturebot @toddbaert
chore(main): release web-sdk 1.1.0 (#929)
ee08911 
🤖 I have created a release *beep* *boop*
---


##
[1.1.0](web-sdk-v1.0.3...web-sdk-v1.1.0)
(2024-05-13)


### ✨ New Features

* set context during provider init on web
([#919](#919))
([7e6c1c6](7e6c1c6))


### 🐛 Bug Fixes

* remove export of OpenFeatureClient
([#794](#794))
([3d197f2](3d197f2))
* removes exports of OpenFeatureClient class and makes event props
readonly ([#918](#918))
([e9a25c2](e9a25c2))
* run error hook when provider returns reason error or error code
([#926](#926))
([c6d0b5d](c6d0b5d))
* skip reconciling event for synchronous onContextChange operations
([#931](#931))
([6c25f29](6c25f29))


### 🧹 Chore

* **main:** release core 1.2.0
([#927](#927))
([692ad5b](692ad5b))


### 📚 Documentation

* add tip about supported usage in the install section
([#941](#941))
([f0de667](f0de667))

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

Signed-off-by: OpenFeature Bot <109696520+openfeaturebot@users.noreply.github.com>
Signed-off-by: Todd Baert <todd.baert@dynatrace.com>
Co-authored-by: Todd Baert <todd.baert@dynatrace.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukas-reining lukas-reining lukas-reining approved these changes

@toddbaert toddbaert toddbaert approved these changes

@weyert weyert Awaiting requested review from weyert

@jonathannorris jonathannorris Awaiting requested review from jonathannorris

@thomaspoignant thomaspoignant Awaiting requested review from thomaspoignant

@maxveldink maxveldink Awaiting requested review from maxveldink

@federicobond federicobond Awaiting requested review from federicobond

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Error hooks only run if the provider throws
6 participants
@beeme1mr @toddbaert @Kavindu-Dodan @federicobond @maxveldink @lukas-reining