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

Bug: validateResponse fails on impression_data #226

Closed
1 task done
10111282 opened this issue Sep 17, 2024 · 3 comments
Closed
1 task done

Bug: validateResponse fails on impression_data #226

10111282 opened this issue Sep 17, 2024 · 3 comments
Assignees
Labels
bug Something isn't working

Comments

@10111282
Copy link

10111282 commented Sep 17, 2024

Is there an existing issue for this?

  • I have searched the existing issues

Describe the bug

Our Unleash setup is the following:
unleash-client-php: v2.5.283
unleash edge: v19.2.0
unleash server: 6.0.6

Currently we use Unleash Edge v18.0.1, it works as expected. We are trying to upgrade Edge to v19.2.0 which causes unleash-client-php (v2.5.283) to always return null in validateResponse(). The reason is that Unleash Edge v19.2.0 returns the response with a bit different fields, in particular it returns impressionData instead of impression_data (Unleash Edge v18.0.1).

Unleash client is configure like this:

  UnleashBuilder::create()
      ->withAppName(config('services.unleash.app.name'))
      ->withAppUrl(config('services.unleash.edge.url'))
      ->withInstanceId(config('services.unleash.edge.srv'))
      ->withProxy(config('services.unleash.edge.key'))
      ->withCacheTimeToLive(30)
      ->withStaleTtl(300)
      ->build()

While debugging it I can see that the request is sent to /api/frontend/features/<feature name>

unleash-client-validation

To reproduce

  1. Invoke $unleash->isEnabled('some-feature-name');
  2. Expected to be true if the feature is enabled, but result is always false.

Sample code (optional)

No response

Version

v2.5.283

Expected behavior

Within validateResponse() it must expect impressionData instead of impression_data, or both for backward compatibility.

Logs (optional)

No response

Additional context (optional)

No response

@gastonfournier
Copy link
Contributor

Hi! This was a fix from Unleash Edge 19.0.0 and it's one of the reasons of the major version bump. The problem is that PHP has implemented it wrong and looked at the snake case version:

$this->impressionData = $response['impression_data'];

I believe a simple solution would be to expect either snake case or camel case and support both for some versions. We can release that as a patch of the current version so the upgrade should be simple

@pallimalilsuhail
Copy link

Hello @gastonfournier. I see you fixed and merged here 1 hour back. when can we expect the release? Thanks a lot.

@gastonfournier
Copy link
Contributor

Hi @pallimalilsuhail the release is on its way, it's going to be 2.6.x

@gastonfournier gastonfournier moved this from Investigating to Done in Issues and PRs Oct 3, 2024
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
Archived in project
Development

Successfully merging a pull request may close this issue.

4 participants