Hello,

The mailchimp module uses the HTTP client to fetch data from mailchimp API.
Usually the Drupal HTTP Client does not need much customization in it's configuration, but for some specific installs, with proxies or specific timeout need, the module does not allow to configure properly.

I suggest a simple modification for allowing it : replacing the creation of a new HTTPClient object in Mailchimp class, by the Drupal 8 service "http_client" that can be configured on a global basis.

Patch would look like something like the one I uploaded (I'll redo it later as a proper patch)

Comments

Renrhaf created an issue. See original summary.

renrhaf’s picture

StatusFileSize
new1.73 KB
renrhaf’s picture

Status: Active » Needs review
agoradesign’s picture

Status: Needs review » Needs work
Related issues: +#2845950: Install Mailchimp PHP API via Composer

Manipulating the shipped Mailchimp API library is not the way we should go, especially as the fact, that the library itself is shipped with this module is the wrong approach anyway, see #2845950: Install Mailchimp PHP API via Composer

If we had a newer version of the library (the shipped one is 1.0.3, the current version on Github is 1.0.6), we would have the possiblity to better configure the HTTP client. Instead of timeout we could pass the whole options array.

As long as the Mailchimp API library does not support to set/inject the HTTP client from outside, imho the best approach would be to load the Drupal "http_client" service within mailchimp_get_api_object(), load its configuration by calling getConfig() and use that as third parameter for instantiating the Mailchimp object. Precondition is of course that we can use a newer library version

agoradesign’s picture

Or should we postpone this issue, depending on #2845950: Install Mailchimp PHP API via Composer and rise the other issue's priority instead?

renrhaf’s picture

You're right, it seems that the issue #2845950 should be dealt with before the one.

agoradesign’s picture

Now, that version 1.0.8 already ships with the module, the topic of using Composer or not is no longer relevant to this issue.

I've looked into the constructor of the Mailchimp class:

public function __construct($api_key, $api_user = 'apikey', $http_options = [], MailchimpHttpClientInterface $client = NULL)

We see, that there's the MailchimpHttpClientInterface, which default Guzzle based implementation (MailchimpGuzzleHttpClient) creates its own Guzzle client within its constructor.

What I'd suggest, is:

  • Implementing our own MailchimpHttpClientInterface class in the module
  • Define it as a Drupal service and inject Drupal's http client
  • Inherit from MailchimpGuzzleHttpClient, only needing to override it's constructor to let Drupal inject the http client instead
  • Instantiate the Mailchimp object with that client instead of only the timeout in mailchimp_get_api_object()

Hopefully, I gonna find time to create a patch

renrhaf’s picture

Suggestion seems fine ! I'm struck with older versions of MailChimp till this bug is resolved... I'll try to look at it too when I have some time.

denix’s picture

StatusFileSize
new1.23 KB

Hi guys,with the last version of the APIs I have been able to simply add the proxy configuration without modifying any library. Here is a patch that it is working for me, based on this issue: https://github.com/thinkshout/mailchimp-api-php/pull/33.

One issue we have now is when the module try to get the list values from the cache instead than from the api, in that case $result is NULL and an error is generated.

Let me know if the patch works for you too and if it is a good solution.
Denis

agoradesign’s picture

Yes, but there's also now the possiblity to add a http client object as well. And we do have a central Guzzle based service for this in Drupal. So why shouldn't we take advantage of this?

If you ask me, the only valid argument against that would be to be able to have a completely different configuration then the default client. But then we need configurable settings for this because hardcoding any value like timeout is more restrictive.

Thinking further, the premium solution would be the described Http client service that allows to override configuration values via settings. Then we would have full customization power because we would have settings on the one hand, but would also be able to fully swap the service with a custom implementation as well.

denix’s picture

Hei @agoradesign, your solution is fine and I agree it should be the right solution in the long term. We could even liaise with the devs of the php wrapper and find an common solution.
My patch is just for people that are in my situation: a corporate proxy, D8 and MC not working; until we can get something more stable and reliable.
Thanks in advance for your time and looking forward your patch!

agoradesign’s picture

Sorry, if you did get me wrong. Your patch is definitely an improvement. It's good to have it here in the queue to enable other people to use it. I would even agree that it would be a good idea to commit this improvement.

I justed wanted to state, what I think that would be the more flexible solution and more the "Drupal way". If we don't discuss it now, we won't discuss it ever

renrhaf’s picture

Assigned: renrhaf » Unassigned
progzy’s picture

@denix : maybe I am missing something but why don't you use "http_client_config" setting (from settings.php) to get proxy configuration ?

I've created another issue for 1.6 version : https://www.drupal.org/project/mailchimp/issues/2951089

Problem is a little bit different since there is a WSOD

denix’s picture

@progzy because simply it does not work :)

With the patch I did and

$settings['proxy_server'] = 'proxy.company.com';
$settings['proxy_port'] = 8080;
$settings['proxy_exceptions'] = array('127.0.0.1', 'localhost');

It works simply fine.

progzy’s picture

@denix does the solution I provide at https://www.drupal.org/project/mailchimp/issues/2951089 works for you? It uses the "http_client_config" setting.

xenophyle’s picture

Status: Needs work » Closed (won't fix)

Thanks for submitting this ticket — and sorry we didn’t get back to you about it sooner. We are cleaning up old issues and issue for old versions and are going to close this one. If this issue is still relevant for you, please re-open it and update the “Version” field, and include any updates to the behavior you are seeing.With a smaller issue queue, we’ll have an easier time getting back to you.