Custom logger ignored by constructor / errors appear in output

[cleentfaar]

Optimizely::__construct() allows us to pass a logger that will be used to log any errors detected by the SDK.

That's great but for some strange reason the constructor overrides the logger with it's own DefaultLogger whenever a validation/connection error occurs; making the custom logger completely useless.

To make matters worse, the DefaultLogger uses the standard php://output stream to send it's messages, causing every site to leak error messages to it's users whenever a connection issue occurs (e.g. the outage last week).

[aliabbasrizvi]

@cleentfaar If you look at the validation logic, you would see that the logger passed in itself could be wrong and in that case we fall back to the DefaultLogger.

Let me think about this more and look at your PR, but I think this problem would go away if all your data is valid.

[cleentfaar]

What do you mean with a wrong logger? According to your constructor, it must implement the LoggerInterface or be null, in which case the NoOpLogger is used, right? I don't see how a logger can be 'wrong' here.

You could improve ProjectConfig's constructor by typehinting it's arguments, but that is only for theoretical purposes; it could never get the wrong logger with the way Optimizely class is currently setup.

I also don't see why a 'wrong' logger should (silently!) fail; clearly the user has implemented it incorrectly and should be notified of that (throw exception). Silencing every (fundamental) error because you rely on a DefaultLogger is not the way to go imo.

[aliabbasrizvi]

@cleentfaar I see your point. Will review your change more closely.

Thanks for the explanation.

[mikeproeng37]

This might no longer be an issue. Will close out. Please re-open if needed.