Skip to content
This repository has been archived by the owner on Jun 2, 2020. It is now read-only.

DEBUG_SDK checking if defined, not the defined value #32

Closed
jonathanlundstrom opened this issue May 21, 2019 · 2 comments
Closed

DEBUG_SDK checking if defined, not the defined value #32

jonathanlundstrom opened this issue May 21, 2019 · 2 comments

Comments

@jonathanlundstrom
Copy link

Describe the bug
We're implementing the Klarna Checkout SDK for one of our customer projects and noticed that we were repeatedly getting a lot of debug information through the requests even though we had defined DEBUG_SDK as false. When we went into the code we could out why. You're correctly using getenv to check if the value exists and get the value, but you're then only checking if the constant is actually defined, not it's value.

The issue is at this line:

$debug = getenv('DEBUG_SDK') || defined('DEBUG_SDK');

Which service do you use
Klarna Checkout

To Reproduce
Steps to reproduce the behavior:

  1. Define the DEBUG_SDK constant as false as below:
    <?php define('DEBUG_SDK', false); ?>
  2. Make a request to the API, for instance create a new Order.

Expected behavior
Klarna SDK debug output should not show.

@alexions
Copy link
Contributor

Hi @jonathanlundstrom

This behaviour was made intentionally. The problem with the environment variables - they are always strings and must be converted to bool before use.

So once getting the value you should guess, what is it? Should it be "false"/"true"? "Y"/"N"? "1"/"0"? How to react on "blablabla" inside the variable or "null"

Too many variants. Of course - there is a function filter_var with FILTER_VALIDATE_BOOLEAN option, but this function has some amount of issues, like these:
https://bugs.php.net/bug.php?id=49510
https://bugs.php.net/bug.php?id=51344

Some of them have status Closed, some Won't fix, but anyway - the reason why we decided to not to parse the input - just to simplify the checking function and avoid possible issues with type casting.

I agree, that this can confuse and probably I need to change the README and add more details about the DEBUG_SDK flag.

Thank you for the feedback!

@alexions
Copy link
Contributor

I've updated the README file in order to remove the confusion about the flag usage. Thank you for your feedback!

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants