Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Error: Future resolved more than once #5

Open
cope opened this issue Aug 26, 2015 · 37 comments
Open

Error: Future resolved more than once #5

cope opened this issue Aug 26, 2015 · 37 comments

Comments

@cope
Copy link

cope commented Aug 26, 2015

So, I get a "User successfully retrieved from LDAP server", but then right after the below error :(

I am using:

LDAP.generateSettings = function (request) {
    return {
        "serverDn": "DC=mycompany,DC=com",
        "serverUrl": "ldap://111.111.111.11",
        "whiteListedFields": ["displayName", "mail", "memberOf"],
        "autopublishFields": ["displayName", "mail", "memberOf"]
    };
}

And this is the error:

I20150826-13:45:12.114(2)? User successfully retrieved from LDAP server
W20150826-13:45:12.164(2)? (STDERR)
W20150826-13:45:12.165(2)? (STDERR) C:\Users\cope\AppData\Local\.meteor\packages\meteor-tool\1.1.4\mt-os.win_32\dev_bundle\server-lib\node_modules\fibers\future.js:226
W20150826-13:45:12.165(2)? (STDERR)                     throw new Error('Future resolved more than once');
W20150826-13:45:12.165(2)? (STDERR)                           ^
W20150826-13:45:12.165(2)? (STDERR) Error: Future resolved more than once
W20150826-13:45:12.165(2)? (STDERR)     at Object.Future.return (C:\Users\cope\AppData\Local\.meteor\package-tool\1.1.4\mt-os.windows.x86_32\dev_bundle\server-lib\node_modules\fibers\future.js:226:10)
W20150826-13:45:12.165(2)? (STDERR)     at EventEmitter.<anonymous> (packages/babrahams:accounts-ldap/ldap_serve:1)
W20150826-13:45:12.165(2)? (STDERR)     at EventEmitter.emit (events.js:95:17)
W20150826-13:45:12.166(2)? (STDERR)     at _done (C:\Users\cope\AppData\Local\.meteor\packages\babrahams_accap\0.3.1\npm\node_modules\ldapjs\lib\client\client.js:785:22)
W20150826-13:45:12.166(2)? (STDERR)     at messageCallback (C:\Users\cope\AppData\Local\.meteor\packages\babccounts-ldap\0.3.1\npm\node_modules\ldapjs\lib\client\client.js:858:14)
W20150826-13:45:12.166(2)? (STDERR)     at Parser.onMessage (C:\Users\cope\AppData\Local\.meteor\packages\baaccounts-ldap\0.3.1\npm\node_modules\ldapjs\lib\client\client.js:199:12)
W20150826-13:45:12.166(2)? (STDERR)     at Parser.emit (events.js:95:17)
W20150826-13:45:12.166(2)? (STDERR)     at Parser.write (C:\Users\cope\AppData\Local\.meteor\packages\babrahunts-ldap\0.3.1\npm\node_modules\ldapjs\lib\messages\parser.js:105:8)
W20150826-13:45:12.166(2)? (STDERR)     at end (C:\Users\cope\AppData\Local\.meteor\packages\babrahams_accou\0.3.1\npm\node_modules\ldapjs\lib\messages\parser.js:71:19)
W20150826-13:45:12.167(2)? (STDERR)     at Parser.write (C:\Users\cope\AppData\Local\.meteor\packages\babrahunts-ldap\0.3.1\npm\node_modules\ldapjs\lib\messages\parser.js:107:10)
=> Exited with code: 8
@JackAdams
Copy link
Owner

This was an issue previously (#3), and I thought I'd fixed it, but apparently not.
:( indeed.

@cope
Copy link
Author

cope commented Aug 26, 2015

Let me know if there is anything I can do to help... I am using 0.3.1:

babrahams:accounts-ldap        added, version 0.3.1
babrahams:accounts-ldap-ionic  added, version 0.1.8
sacha:spin                     added, version 2.3.1

@JackAdams
Copy link
Owner

I had a quick look through the code and couldn't spot how any of the Futures could be resolving twice, but I have to admit - I'm pretty shaky when it comes to Fibers and Futures.

If you'd like to take a look at the code in this part of the codebase, that's probably where the error is occurring.

@cope
Copy link
Author

cope commented Aug 26, 2015

I am a total n00b when it comes to Futures :( Never used it so far.

Do you need a for loop on line 115? What if you just have:

//for (var k in serverDNs) {
    var serverDn = serverDNs[0].split(/,?DC=/).slice(1).join('.');

    ...

    success = bindFuture.wait();
    // if (success) { break; }
//}

?

@JackAdams
Copy link
Owner

Yeah, that's there so you can put an array in as the value for serverDn. For the settings you're generating, that for loop is only going to run once, so it wouldn't make any difference.

@cope
Copy link
Author

cope commented Aug 26, 2015

I thought so too...

Question, is client.bind asynchronous?

@JackAdams
Copy link
Owner

Yep. client.bind is async. So is client.search. That's why there's a need for Futures.

@cope
Copy link
Author

cope commented Aug 26, 2015

And then here then

success = bindFuture.wait();

the code waits for the asynchronous call to return the value?

Then I am almost 100% sure that the problem is NOT in _bind.

Which would mean it is below in _search where searchFuture is used?

Is it possible for the client.search call to fire two events? Maybe "searchEntry" and "searchReference" events? OR, and this seems more likely to me, "error" and "end" events one after another? Or the "end" event after any other event?

@cope
Copy link
Author

cope commented Aug 26, 2015

WAIT! Notice above, I get: "User successfully retrieved from LDAP server", which means we are looking in the wrong place, aren't we? Line 299:

LDAP.log("User successfully retrieved from LDAP server");

which is well after _bind and _search finished, right?

@JackAdams
Copy link
Owner

Okay. I'm glad you persisted with this. I'm also pretty sure that _bind is not the problem and that, like you say, there is a possibility that the _search Future resolves twice - once on something (not sure which yet) and then again on end. The first resolution lets the rest of the code run, so the function returns and the code down around line 299 runs. Then, on end, it tries to resolve again, finds that that ship has already sailed, and throws an error.

My guess is that an empty userObj is being "successfully" retrieved from the LDAP server on searchEntry, which causes searchFuture to return, and then on end, we find userObj is empty and searchFuture returns again with false (so now it has resolved twice, leading to the error).

I have a rough and ready fix sketched out in my head, but I don't think I'll have time to test and release a new version today.

@cope
Copy link
Author

cope commented Aug 26, 2015

OK, so bad news, my error log in the OP was somehow cut-off at the worse line, the good news is that I have the new log and it shows exactly where the problem is!

W20150826-15:39:25.549(2)? (STDERR) Error: Future resolved more than once
W20150826-15:39:25.549(2)? (STDERR)     at Object.Future.return (C:\Users\cope\AppData\Local\.meteor\packages\meteor-tool\1.1.4\mt-os.windows.x86_32\dev_bundle\server-lib\node_modules\fibers\future.js:226:10)
W20150826-15:39:25.549(2)? (STDERR)     at EventEmitter.<anonymous> (packages/babrahams:accounts-ldap/ldap_server.js:180:1)

As you can see, the line is 180, which is:

searchFuture.return(false);

under searchReference event:

res.on('searchReference', function (referral) {
    LDAP.log('referral: ' + referral.uris.join());
    searchFuture.return(false);
});

@cope
Copy link
Author

cope commented Aug 26, 2015

Today is NOT an issue :) No worries, I can live without it :D

I just started with Meteor like a week ago, and I wanted to build an app to convince my boss that Meteor is the way to go from now on :D And for that I needed an LDAP login...

The good news is that my boss is on vacation for another 10 days, so I have time :D

@JackAdams
Copy link
Owner

Ah ... then my guess above was wrong, but this is very helpful info!
I'll try to release a new version in the next day or two with a fix, but I'll need a bit of time to replicate the error in a dev environment first.

@cope
Copy link
Author

cope commented Aug 26, 2015

Whenever you release it, I'll be here to test it :)

Thanks!

@JackAdams
Copy link
Owner

Cool. Thanks for your help with this. Very much appreciated.

@cope
Copy link
Author

cope commented Aug 27, 2015

Just thinking out loud here, and I appologize if this is a dumb question...

but, is there such a thing as future.unwait or unbind, or something that will tell the future object it should not listen anymore?

@JackAdams
Copy link
Owner

I'd have to read up on the docs about Futures. I think I should do that anyway ... 😁

Sent from my iPhone

On Aug 27, 2015, at 4:36 PM, Predrag Stojadinovic [email protected] wrote:

Just thinking out loud here, and I appologize if this is a dumb question...

but, is there such a thing as future.unwait or unbind, or something that will tell the future object it should not listen anymore?


Reply to this email directly or view it on GitHub.

@cope
Copy link
Author

cope commented Aug 27, 2015

:D Can you live a link to those docs here as well, googling with the word "future" just gives back stupid results :(

@JackAdams
Copy link
Owner

https://github.com/laverdet/node-fibers

Just noticed that there is an isResolved() API call. That should solve all our problems. I'll try to release a new version in the next hour or two.

@JackAdams
Copy link
Owner

Okay. Just released 0.3.2. Will be interested to hear what happens.

@cope
Copy link
Author

cope commented Aug 27, 2015

So, it seems that the Future issue is resolved. After "User successfully retrieved from LDAP server" it prints a whole bunch of text starting with {"dn":"CN=...

But then throws this error:

I20150827-13:59:59.635(2)? Exception while invoking method 'login' TypeError: Cannot read property '' of null
I20150827-13:59:59.635(2)?     at [object Object].Package (packages/babrahams:accounts-ldap/ldap_server.js:320:1)
I20150827-13:59:59.635(2)?     at packages/accounts-base/accounts_server.js:358:1
I20150827-13:59:59.635(2)?     at tryLoginMethod (packages/accounts-base/accounts_server.js:161:1)
I20150827-13:59:59.636(2)?     at runLoginHandlers (packages/accounts-base/accounts_server.js:355:1)
I20150827-13:59:59.636(2)?     at [object Object].Meteor.methods.login (packages/accounts-base/accounts_server.js:409:1)

I20150827-13:59:59.636(2)?     at maybeAuditArgumentChecks (packages/ddp/livedata_server.js:1617:1)
I20150827-13:59:59.636(2)?     at packages/ddp/livedata_server.js:648:1
I20150827-13:59:59.640(2)?     at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
I20150827-13:59:59.640(2)?     at packages/ddp/livedata_server.js:647:1
I20150827-13:59:59.640(2)?     at [object Object]._.extend.withValue (packages/meteor/dynamics_nodejs.js:56:1)
I20150827-13:59:59.640(2)? referral: ldap://DomainDnsZones.mycompany.com/DC=DomainDnsZones,DC=mycompany,DC=com
I20150827-13:59:59.640(2)? referral: ldap://ForestDnsZones.mycompany.com/DC=ForestDnsZones,DC=mycompany,DC=com
I20150827-13:59:59.641(2)? referral: ldap://nvprod.mycompany.com/DC=nvprod,DC=mycompany,DC=com
I20150827-13:59:59.641(2)? referral: ldap://mycompany.com/CN=Configuration,DC=mycompany,DC=com
I20150827-13:59:59.641(2)? status: 0

It says ldap_server.js:320 but that line is commented out?

@JackAdams
Copy link
Owner

You're getting a referral there. This is something the package won't help with. I don't think it's going to be able to retrieve a record from your LDAP server unless you change the settings you're using.

I've just released 0.3.3 which doesn't throw an error in this situation, but still doesn't solve your problem.

@cope
Copy link
Author

cope commented Aug 27, 2015

Crap :( I have no idea where the referral comes from :(

I am using these settings:

LDAP.generateSettings = function (request) {
    return {
        "serverDn": "DC=mycompany,DC=com",
        "serverUrl": "ldap://111.11.111.11",
        "whiteListedFields": ["displayName", "userprincipalname", "mail", "memberOf"],
        "autopublishFields": ["displayName", "userprincipalname", "mail", "memberOf"]
    };
}

@cope
Copy link
Author

cope commented Aug 27, 2015

Question, given that this happens:

"User successfully retrieved from LDAP server"

is it possible to, well, stop there?

What I mean is, the user and pwd are confirmed, so what could be causing those referrals? :(

@JackAdams
Copy link
Owner

Does the latest version (0.3.3) still say

"User successfully retrieved from LDAP server"

and then fail?

@cope
Copy link
Author

cope commented Aug 29, 2015

I'll have to recheck on Monday at work, but looking at the comments above, I did update after you announced 0.3.3, so I would say yes.

@JackAdams
Copy link
Owner

Weird. And the data that immediately follows User successfully retrieved from LDAP server looks like a correct record from the LDAP server, complete with username and email fields?

@cope
Copy link
Author

cope commented Aug 29, 2015

Exactly. (again, I will recheck on Monday to confirm this properly)

@JackAdams
Copy link
Owner

Hmm... I'm running out of ideas.

If you want to play around with this a bit, to see if you can isolate exactly what's going on, you could clone this repo into a /packages directory in the root directory of your app and then that local version will override my version from atmosphere until you delete the cloned-and-modified package from the /packages directory. That will allow you to make changes, test and troubleshoot locally. And if you find a solution I'll be more than happy to merge the changes back into master. In fact, I'll be really, really grateful. But no pressure -- it's just a way you can take the situation into your own hands if you want to. :-)

@cope
Copy link
Author

cope commented Aug 29, 2015

Not a bad idea. I will try that first thing on Monday, then.

@cope
Copy link
Author

cope commented Sep 1, 2015

Sorry for not replying yesterday, bugs happened, must be taken care of... Hopefully I will have some time during the week to try your suggestion.

@JackAdams
Copy link
Owner

No problem at all. If you can figure it out, that'd be fantastic but, like I said, no pressure from my side -- I see any contributions as an unexpected bonus. You've already done a lot to tighten up the existing codebase by keeping this issue alive and giving feedback on the latest releases.

@JackAdams
Copy link
Owner

Sorry for being so quiet on this issue for so long. I've released 0.4.6 today, which may or may not help. In any case, I'm grateful for your encouragement in debugging this package.

@skarborg
Copy link

This was happening to me when my filter was too "wide" (objectClass=user) resulting in many matches and callbacks from ldapjs client. With a proper search filter (&(CN=username)(objectClass=user)) and one match/result this stopped happening.

@skarborg
Copy link

skarborg commented Oct 28, 2016

Shouldnt the

searchFuture.return

in

.on('searchEntry'

also be wrapped with

if (!searchFuture.isResolved()) {
}

?

Thanks for great package btw

@JackAdams
Copy link
Owner

JackAdams commented Oct 28, 2016

Well... that explains a few things. And, yes, it should be wrapped as you've suggested. I imagine that would have prevented the future from resolving more than once when the net was cast too wide and it returned multiple matches.

The thanks really belong to someone else. I just took hive:accounts-ldap and kept modifying it in response to my app's needs. I don't actually know a heck of a lot about futures, LDAP, etc.

@catonice
Copy link

Good day peeps,
I encountered this error "Future resolve more than once" and tried a lot of things to fix it.
I found that some specific serverDns would cause this error to be thrown and this comment by @skarborg gave me that clue to selectively test each one (I can run against multiple serverDns but in my specific case a few were the culprits):
"This was happening to me when my filter was too "wide" (objectClass=user) resulting in many matches and callbacks from ldapjs client. With a proper search filter (&(CN=username)(objectClass=user)) and one match/result this stopped happening."
I removed the problem dns in my case (I don't have a lot of knowledge on what dns we are using so).
I hope this helps someone else.
Thanks!

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

No branches or pull requests

4 participants