Ldaprecord-laravel: ldap_bind error when used in background worker

[AlexH269]

Describe the bug
The LDAP connection is initalized as soon as the Connection object is constructed:
https://github.com/DirectoryTree/LdapRecord/blob/master/src/Connection.php#L102

The connection is constructed by the service provider, while the framework is booting. This is usually not an issue since PHP requests are short lived and every requests boots a new copy of the framework. Since Laravel jobs using queues are a long lived, I ran into ldap_bind errors after the queue worker is running for about an hour. This is due to the fact, that the framework is only booted once for the queue worker, therefor only connecting the LDAP once. When this connection times out, subsequent LDAP queries fail:

[previous exception] [object] (ErrorException(code: 2): ldap_bind(): Unable to bind to server: Can't contact LDAP server at /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Ldap.php:583)
[stacktrace]
#0 [internal function]: LdapRecord\\Ldap->LdapRecord\\{closure}(2, 'ldap_bind(): Un...', '/var/www/projec...', 583, Array)
#1 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Ldap.php(583): ldap_bind(Resource id #1442, 'username', 'password')
#2 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Ldap.php(889): LdapRecord\\Ldap->LdapRecord\\{closure}()
#3 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Ldap.php(584): LdapRecord\\Ldap->executeFailableOperation(Object(Closure))
#4 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Auth/Guard.php(118): LdapRecord\\Ldap->bind('password', 'password')
#5 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Auth/Guard.php(142): LdapRecord\\Auth\\Guard->bind('password', 'password')
#6 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Connection.php(202): LdapRecord\\Auth\\Guard->bindAsConfiguredUser()
#7 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Connection.php(259): LdapRecord\\Connection->connect()
#8 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(533): LdapRecord\\Connection->run(Object(Closure))
#9 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(398): LdapRecord\\Query\\Builder->run('(&(objectclass=...')
#10 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(503): LdapRecord\\Query\\Builder->LdapRecord\\Query\\{closure}()
#11 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(401): LdapRecord\\Query\\Builder->getCachedResponse('(&(objectclass=...', Object(Closure))
#12 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(224): LdapRecord\\Query\\Builder->query('(&(objectclass=...')
#13 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(246): LdapRecord\\Query\\Builder->LdapRecord\\Query\\{closure}()
#14 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(225): LdapRecord\\Query\\Builder->onceWithColumns(Array, Object(Closure))
#15 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(699): LdapRecord\\Query\\Builder->get(Array)
#16 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Builder.php(717): LdapRecord\\Query\\Builder->first(Array)
#17 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Model/Builder.php(199): LdapRecord\\Query\\Builder->firstOrFail(Array)
#18 /var/www/projects/pswain/vendor/directorytree/ldaprecord/src/Query/Model/Builder.php(173): LdapRecord\\Query\\Model\\Builder->findByGuidOrFail('\\\\5b\\\\9f\\\\40\\\\1b\\\\ea...', Array)

To Reproduce
Steps to reproduce the behavior:

1. Configure a simple job which makes an LDAP query:

<?php

namespace App\Jobs;

use Illuminate\Bus\Queueable;
use Illuminate\Contracts\Queue\ShouldQueue;
use Illuminate\Foundation\Bus\Dispatchable;
use Illuminate\Queue\InteractsWithQueue;
use Illuminate\Queue\SerializesModels;
use LdapRecord\Models\ActiveDirectory\User;

class LdapDemo implements ShouldQueue
{
    use Dispatchable, InteractsWithQueue, Queueable, SerializesModels;

    /**
     * Execute the job.
     *
     * @return void
     */
    public function handle()
    {
        User::where('samaccountname', '=', 'demo')->get();
    }
}

````

2. Start the queue worker

php artisan queue:work

3. Dispatch the job, which should successfully run.

4. Wait for about an hour (I'm not sure exactly how long it takes)

5. Dispatch the job again and it will fail with the exception above.

6. If the following code is updated (just as a workarround) it works:
https://github.com/DirectoryTree/LdapRecord/blob/master/src/Models/Model.php#L291
```php
public function newQueryWithoutScopes()
{
    $connection = static::resolveConnection($this->connection);
    $connection->reconnect();
    return $connection->query()->model($this);
}

Expected behavior
I suggest, that we don't connect the LDAP on the connection objects construction, but rather when it is pulled from the container.

Environment):

• LDAP Server Type: ActiveDirectory
• PHP Version: 7.3.3
• Laravel Version: 6.0

[stevebauman]

Hey @AlexH269, you're right, this needs to be fixed.

Right now I'm running the queue:listen command instead of queue:work to bypass this problem, but my jobs are much slower due to this.

I'm wondering how Laravel handles re-establishing a database connection dynamically during long running queues... Maybe we can take advantage of the __wakeup() and __sleep() magic methods to call reconnect() on connections when model are queried?

[AlexH269]

queue:listen will reboot the framework for every job, so in a sense it works just like a HTTP request. I will check how laravel handles the database connection and report back here :)

[AlexH269]

Looks like you already found the problem previously:

https://github.com/Adldap2/Adldap2-Laravel/issues/289#issuecomment-567635061

[stevebauman]

Ah yes you're right, it's been a while! I have this reconnection implementation in LdapRecord, but it was changed recently:

https://github.com/DirectoryTree/LdapRecord/commit/81eb7293f32012bb1c0e01d5e4a57d43bdc79cf8

I'm guessing you only have one host specified in your configuration?

[AlexH269]

Correct. I only use one host in my Dev enviroment, but usually two or more in production.

[stevebauman]

Ah okay great, that makes sense. So I'll have to extend the 'retry' functionality to first retry on the initial host, then move onto the next.

I think what we can do is:

• If only one host is specified in the configuration, retry the operation on that host
• If two or more hosts are specified, retry on the next host
  
  
  
  • If the next host is successful, reset the attempted array so it starts the process over on next failure
  
  

Do you see any issues / potential issues with this?

[AlexH269]

I'm wondering if it would be best, to always try the first host again. Otherwise we might see Queue Jobs quering a different controller than the rest of the application (usually) uses.

HTTP request:

• Boots framework
• Gets working connection for first server

Queue Job:

• Framework is booted one time
• Connection eventually times out
• Gets working connection for the second server

In some environments, this might also cause issues due to replication (e.g. both configured hosts not holding the same data at the same time).

I think it would be best to always use the first server and only move on, if it is really not available. It's kind of difficult, because it doesn't look like we can differentiate between the host actually not being reachable and a timed out connection.

If we document the behavior, that you describe it is probably fine.

[stevebauman]

In some environments, this might also cause issues due to replication (e.g. both configured hosts not holding the same data at the same time).

I think it would be best to always use the first server and only move on, if it is really not available.

Yes you're right, definitely have to take this into account - I agree, we should retry the primary server first, then move onto the next (if available).

Going to give this a shot right now and see what I can come up with!

[stevebauman]

Okay I think we're golden! Whenever you have time, would you perhaps be able to give the latest dev-master a shot? Primary servers will now be retried once before moving onto the next host in line.

However, I'm unsure if I have to add a __wakeup or __sleep call to reset / empty out the attempted property on the LdapRecord Connection class, as I'm not sure if it will be stored in application state in the queue worker?

[AlexH269]

I had to fight composer a bit due to version constraints with the ldaprecord-laravel package, but this worked for me:

composer require "directorytree/ldaprecord:dev-master as 1.5"

I tested it after a few hours and it works perfectly :)

I did some logging and the attempted property seems to be cleared correctly. I will check it out in more detail later today though.

[stevebauman]

Perfect, thanks @AlexH269! Really appreciate it, you've been a great help.

I'll make a new release shortly 😄

[stevebauman]

v1.6.0 is out with this patch! :tada:

Keep your feedback coming @AlexH269! You've been killin it! 👍

[AlexH269]

You are welcome, thanks for all your work.

Looks like everything is fine with the $attempted property, but once i'm done implementing the library, I will check if it works longterm in my staging environment.