Revert "added Guzzle options to prevent PHP fatal error"

[AndyGaskell]

Reverts #26

I've realised I didn't really think this through, it would probably benefit from a different approach

[rolinger]

Is the intent to revert until the documentation is provided? Or were there breaking changes that forced the revert?

[AndyGaskell]

I was thinking it through again, and thought that maybe it might be better to do it in the client creation method, that you mentioned in...
#17 (comment)

[AndyGaskell]

So, my intent was to discuss it with you, and if you agree with the aproach, try and code up a backward compatible implementation that set the guzzle options in the client creation.

I'm just trying to think through the use cases really.

[rolinger]

I think it should be at connection level, not a global setting. There could be use cases where you want it to error out and others where you don't want it to error out (ie: deviceInfo). I also think it should be set to false (IE: do not error out) be default, and then the user should forcefully set it true if they intend for it to error out.

In our implementation of FCM, the majority of scripts are going to want to set HTTP error out to false, otherwise it breaks our scripts that are looping through large sets of user/deviceIDs.

[rolinger]

...but it need to thoroughly documented on the behavior, esp for people who are used to or expecting Guzzle HTTP erroring out - and for those who need to enable it.

[EdwinHoksberg]

My 2 cents: we have a getGuzzleClient method, maybe we can add a setGuzzleClient? That way the user can provide their own client and built it with all their required options/headers/etc..

[rolinger]

might make it to complicated...and who knows what settings they use or how it could break our PHP-FCM causing us to try and troubleshoot stuff thats not related to our project.

[AndyGaskell]

Thanks for all the thoughts :)

So, are the options...

1. Do it on each call. Not sure, but I guess this would be a change to 10 functions.
2. Do it on the client creation
3. Create a setter function

Option 1 is good, as it allows the user to do different things on various calls, as you mention @rolinger which is nice and flexible. There would be quite a lot of code to change though.

Option 2 is simpler, in terms of code changes I think. If a user wanted to sometimes get errors and sometimes not, they could just create 2 clients, though that maybe sounds a little in-elegant solution.

Option 3 is great as it'd allow us flexibility for the future and folk will understand the getter / setter principles. There would not be too much code to change.

Any other options?...

[AndyGaskell]

Thinking about it, I guess we could do option 2 and option 3.

[rolinger]

I def haven't put as much thought into it as you have. But evaluating what you laid out I would say #2 or #3 - however I don't fully understand what you mean by getter/setter options. What would be an example of that?

[AndyGaskell]

I'll try jot a bit more of a fleshed out plan of the idea in my head.

The getter / setter is a OO design pattern that originates more from Java and JS than PHP. So a class has a property and you can either get the property or set the property. In this instance we have a getGuzzleClient function, so I guess we'd add a setGuzzleClient.

The best quick explanation I found was...
https://www.codejava.net/coding/java-getter-and-setter-tutorial-from-basics-to-best-practices

[AndyGaskell]

I don't think it's a finished solution, and I need to try some real world examples in it, but in the above code I've jotted an idea of a solution.

Features...

You can set options when you create the FcmClient...
$client = new \Fcm\FcmClient($serverKey, $senderId, Array("http_errors" => false) );
...or use the default...
$client = new \Fcm\FcmClient($serverKey, $senderId);

You can set the options in your FcmClient any time...
$client->setGuzzleClient( Array("http_errors" => true) );

The only option at the moment is "http_errors". I guess if we only ever have one option, putting "http_errors" in an array is not very useful. I think other options might be useful in future, like a "invalid_json_error" perhaps.

The setter code is probably not done the best way. Also, I'm not sure if options should be a public property.

AndyGaskell@aa3e20c

[rolinger]

@AndyGaskell - just came back to this here. Too many threads on it. LOL

I think our default should have http_errors => FALSE/OFF...then if the user wants them on they can set them to TRUE/ON. I think most users of php-fcm will not want http_errors cancelling out of their recursive scripts. It also gives the user a chance to manage their own errors.

My vote is:

1. `$client = new \Fcm\FcmClient($serverKey, $senderId) ; // http_errors=false, off by default
2. $client->setGuzzleClient( Array("http_errors" => true) ); // manually switch them back on.

And I would leave it as an array, who knows what else we might need to add as a guzzle parameter latter on.

[rolinger]

@AndyGaskell - when will you merge your new code? I just updated my php-fcm version and need to make certain the guzzle http_errors doesn't trip up my scripts again. I keep having to go in and custom edit the guzzle client.

[AndyGaskell]

Thanks for the feedback @rolinger :) I'll merge this just now.

[AndyGaskell]

@rolinger can you please check it over and merge it if it looks ok
potential solution for guzzle options #31
thanks