Surface ldapjs's real parser/error cause instead of a generic "closed"
Neither raising the client timeout nor switching to paged search fixed the directory-wide import browse — narrowing it down with a plain ldapsearch (same filter, run from inside this exact container, both dn-only and full-attribute) showed the network path and response content are both fine; a standard LDAP client handles it without issue. That leaves ldapjs's own response handling. Its client-level `error` event (e.g. a BER/protocol parser failure) forcibly closes the socket, but we were swallowing that event silently — so every failure surfaced as the same uninformative "<id> closed" ConnectionError regardless of what actually went wrong. Now the last captured client `error` is attached to the bind/search rejection instead, so the UI's error message will show the real underlying cause next time this fails. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012o9j9RezxbZVKQMrB7oRLY
This commit is contained in:
1 parent
a40723b61b
commit
2a505da975
1 file changed
+22
-6
+22
-6
@@ -13,6 +13,13 @@ interface ConnectionSettings {
|
|||||||
useTls: boolean;
|
useTls: boolean;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
// ldapjs's client-level `error` event fires for things like a BER/protocol
|
||||||
|
// parser failure on the response stream — which then also forcibly closes
|
||||||
|
// the socket, so the pending bind/search callback only ever sees a generic
|
||||||
|
// "<id> closed" ConnectionError with no hint of the real cause. Stashing the
|
||||||
|
// last `error` payload per client lets callers report the actual reason.
|
||||||
|
const lastClientError = new WeakMap<ldap.Client, Error>();
|
||||||
|
|
||||||
function createLdapClient(settings: ConnectionSettings, timeoutMs = 5_000): ldap.Client {
|
function createLdapClient(settings: ConnectionSettings, timeoutMs = 5_000): ldap.Client {
|
||||||
const protocol = settings.useTls ? "ldaps" : "ldap";
|
const protocol = settings.useTls ? "ldaps" : "ldap";
|
||||||
const client = ldap.createClient({
|
const client = ldap.createClient({
|
||||||
@@ -26,15 +33,24 @@ function createLdapClient(settings: ConnectionSettings, timeoutMs = 5_000): ldap
|
|||||||
connectTimeout: 5_000,
|
connectTimeout: 5_000,
|
||||||
});
|
});
|
||||||
// A socket-level error (e.g. host unreachable) with no listener would
|
// A socket-level error (e.g. host unreachable) with no listener would
|
||||||
// throw and crash the process — swallow it here, every call site already
|
// throw and crash the process — every call site already handles failure
|
||||||
// handles failure via the bind/search callback's error argument.
|
// via the bind/search callback's error argument, so this only needs to
|
||||||
client.on("error", () => {});
|
// record the error for that fallback, never rethrow.
|
||||||
|
client.on("error", (err) => {
|
||||||
|
lastClientError.set(client, err);
|
||||||
|
});
|
||||||
return client;
|
return client;
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/** Prefers the client's last captured `error` event over a generic connection-closed error, since the former usually carries the real cause. */
|
||||||
|
function describeError(client: ldap.Client, err: Error): Error {
|
||||||
|
const captured = lastClientError.get(client);
|
||||||
|
return captured ?? err;
|
||||||
|
}
|
||||||
|
|
||||||
function bind(client: ldap.Client, dn: string, password: string): Promise<void> {
|
function bind(client: ldap.Client, dn: string, password: string): Promise<void> {
|
||||||
return new Promise((resolve, reject) => {
|
return new Promise((resolve, reject) => {
|
||||||
client.bind(dn, password, (err) => (err ? reject(err) : resolve()));
|
client.bind(dn, password, (err) => (err ? reject(describeError(client, err)) : resolve()));
|
||||||
});
|
});
|
||||||
}
|
}
|
||||||
|
|
||||||
@@ -55,7 +71,7 @@ function search(
|
|||||||
// stream of `searchEntry` events followed by a single `end`.
|
// stream of `searchEntry` events followed by a single `end`.
|
||||||
client.search(baseDn, { filter, scope: "sub", paged: { pageSize: 200 } }, (err, res) => {
|
client.search(baseDn, { filter, scope: "sub", paged: { pageSize: 200 } }, (err, res) => {
|
||||||
if (err) {
|
if (err) {
|
||||||
reject(err);
|
reject(describeError(client, err));
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
res.on("searchEntry", (entry) => {
|
res.on("searchEntry", (entry) => {
|
||||||
@@ -65,7 +81,7 @@ function search(
|
|||||||
}
|
}
|
||||||
results.push({ dn: entry.objectName ?? entry.pojo.objectName, attributes });
|
results.push({ dn: entry.objectName ?? entry.pojo.objectName, attributes });
|
||||||
});
|
});
|
||||||
res.on("error", (searchErr) => reject(searchErr));
|
res.on("error", (searchErr) => reject(describeError(client, searchErr)));
|
||||||
res.on("end", () => resolve(results));
|
res.on("end", () => resolve(results));
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|||||||
Reference in new issue
Block a user