Skip to content

Commit 2992d10

Browse files
committed
Make routing discovery less picky on errors
1 parent a87fd0b commit 2992d10

9 files changed

+97
-21
lines changed

src/v1/internal/connection-providers.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,10 @@ export class LoadBalancer extends ConnectionProvider {
211211
// try next router
212212
return this._createSessionForRediscovery(currentRouter).then(session => {
213213
if (session) {
214-
return this._rediscovery.lookupRoutingTableOnRouter(session, currentRouter);
214+
return this._rediscovery.lookupRoutingTableOnRouter(session, currentRouter).catch(error => {
215+
this._log.warn(`unable to fetch routing table because of an error ${error}`);
216+
return null;
217+
});
215218
} else {
216219
// unable to acquire connection and create session towards the current router
217220
// return null to signal that the next router should be tried

test/internal/connection-providers.test.js

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1117,6 +1117,9 @@ function newLoadBalancerWithSeedRouter(seedRouter, seedRouterResolved,
11171117
loadBalancer._routingTable = new RoutingTable(routers, readers, writers, expirationTime);
11181118
loadBalancer._rediscovery = new FakeRediscovery(routerToRoutingTable);
11191119
loadBalancer._hostNameResolver = new FakeDnsResolver(seedRouterResolved);
1120+
if (expirationTime === Integer.ZERO) {
1121+
loadBalancer._useSeedRouter = false;
1122+
}
11201123
return loadBalancer;
11211124
}
11221125

@@ -1170,7 +1173,7 @@ class FakeRediscovery {
11701173
}
11711174

11721175
lookupRoutingTableOnRouter(ignored, router) {
1173-
return this._routerToRoutingTable[router.asKey()];
1176+
return Promise.resolve(this._routerToRoutingTable[router.asKey()]);
11741177
}
11751178
}
11761179

test/internal/node/routing.driver.boltkit.test.js

Lines changed: 54 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -198,7 +198,7 @@ describe('routing driver with stub server', () => {
198198
// When
199199
const session = driver.session(neo4j.READ);
200200
session.run("MATCH (n) RETURN n.name").catch(err => {
201-
expect(err.code).toEqual(neo4j.error.PROTOCOL_ERROR);
201+
expect(err.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
202202

203203
session.close();
204204
driver.close();
@@ -562,8 +562,6 @@ describe('routing driver with stub server', () => {
562562
// When
563563
const session1 = driver.session(neo4j.session.READ);
564564
session1.run("MATCH (n) RETURN n.name").catch(() => {
565-
const session2 = driver.session(neo4j.session.READ);
566-
session2.run("MATCH (n) RETURN n.name").then(() => {
567565
driver.close();
568566
seedServer.exit(code1 => {
569567
readServer.exit(code2 => {
@@ -572,7 +570,6 @@ describe('routing driver with stub server', () => {
572570
done();
573571
});
574572
});
575-
});
576573
});
577574
});
578575
});
@@ -592,8 +589,8 @@ describe('routing driver with stub server', () => {
592589
const session = driver.session();
593590
session.run("MATCH (n) RETURN n.name").catch(err => {
594591
expect(err.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
595-
expect(err.message.indexOf('Make sure you are connecting to a causal cluster') > 0).toBeTruthy();
596-
assertHasRouters(driver, ['127.0.0.1:9001']);
592+
expect(err.message).toContain('Could not perform discovery');
593+
assertHasRouters(driver, []);
597594
session.close();
598595
driver.close();
599596
server.exit(code => {
@@ -960,31 +957,31 @@ describe('routing driver with stub server', () => {
960957
});
961958
});
962959

963-
it('should throw protocol error when no records', done => {
960+
it('should throw error when no records', done => {
964961
testForProtocolError('./test/resources/boltstub/empty_get_servers_response.script', done);
965962
});
966963

967-
it('should throw protocol error when no TTL entry', done => {
964+
it('should throw error when no TTL entry', done => {
968965
testForProtocolError('./test/resources/boltstub/no_ttl_entry_get_servers.script', done);
969966
});
970967

971-
it('should throw protocol error when no servers entry', done => {
968+
it('should throw error when no servers entry', done => {
972969
testForProtocolError('./test/resources/boltstub/no_servers_entry_get_servers.script', done);
973970
});
974971

975-
it('should throw protocol error when multiple records', done => {
972+
it('should throw error when multiple records', done => {
976973
testForProtocolError('./test/resources/boltstub/unparsable_ttl_get_servers.script', done);
977974
});
978975

979-
it('should throw protocol error on unparsable record', done => {
976+
it('should throw error on unparsable record', done => {
980977
testForProtocolError('./test/resources/boltstub/unparsable_servers_get_servers.script', done);
981978
});
982979

983-
it('should throw protocol error when no routers', done => {
980+
it('should throw error when no routers', done => {
984981
testForProtocolError('./test/resources/boltstub/no_routers_get_servers.script', done);
985982
});
986983

987-
it('should throw protocol error when no readers', done => {
984+
it('should throw error when no readers', done => {
988985
testForProtocolError('./test/resources/boltstub/no_readers_get_servers.script', done);
989986
});
990987

@@ -2125,6 +2122,49 @@ describe('routing driver with stub server', () => {
21252122
});
21262123
})
21272124

2125+
it('should revert to initial router if the only known router returns invalid routing table', done => {
2126+
if (!boltStub.supported) {
2127+
done();
2128+
return;
2129+
}
2130+
2131+
// the first seed to get the routing table
2132+
// the returned routing table includes a non-reachable read-server and points to only one router
2133+
// which will return an invalid routing table
2134+
const seedServer1 = boltStub.start('./test/resources/boltstub/acquire_endpoints_v3_point_to_empty_router.script', 9001);
2135+
// returns an empty routing table
2136+
const failingSeedServer2 = boltStub.start('./test/resources/boltstub/acquire_endpoints_v3_empty.script', 9004);
2137+
// returns a normal routing table
2138+
const seedServer3 = boltStub.start('./test/resources/boltstub/acquire_endpoints_v3_three_servers.script', 9003);
2139+
// ordinary read server
2140+
const readServer = boltStub.start('./test/resources/boltstub/read_server_v3_read_tx.script', 9002);
2141+
2142+
boltStub.run(() => {
2143+
const driver = boltStub.newDriver('bolt+routing://my.virtual.host:8080', {
2144+
resolver: address => ['127.0.0.1:9001', '127.0.0.1:9003']
2145+
});
2146+
2147+
const session = driver.session(neo4j.session.READ);
2148+
session.readTransaction(tx => tx.run('MATCH (n) RETURN n.name')).then(res => {
2149+
session.close();
2150+
driver.close();
2151+
seedServer1.exit(code1 => {
2152+
failingSeedServer2.exit(code2 => {
2153+
seedServer3.exit(code3 => {
2154+
readServer.exit(code4 => {
2155+
expect(code1).toEqual(0);
2156+
expect(code2).toEqual(0);
2157+
expect(code3).toEqual(0);
2158+
expect(code4).toEqual(0);
2159+
done();
2160+
});
2161+
});
2162+
});
2163+
});
2164+
}).catch(error => done.fail(error));
2165+
});
2166+
});
2167+
21282168
function testAddressPurgeOnDatabaseError(query, accessMode, done) {
21292169
if (!boltStub.supported) {
21302170
done();
@@ -2254,7 +2294,7 @@ describe('routing driver with stub server', () => {
22542294

22552295
const session = driver.session();
22562296
session.run('MATCH (n) RETURN n.name').catch(error => {
2257-
expect(error.code).toEqual(neo4j.error.PROTOCOL_ERROR);
2297+
expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
22582298

22592299
session.close();
22602300
driver.close();
Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,9 @@
1+
!: BOLT 3
2+
!: AUTO HELLO
3+
!: AUTO RESET
4+
5+
C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": {}} {}
6+
PULL_ALL
7+
S: SUCCESS {"fields": ["ttl", "servers"]}
8+
RECORD [9223372036854775807, []]
9+
SUCCESS {}
Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
!: BOLT 3
2+
!: AUTO HELLO
3+
!: AUTO RESET
4+
5+
C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": {}} {}
6+
PULL_ALL
7+
S: SUCCESS {"fields": ["ttl", "servers"]}
8+
RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9010"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9011"], "role": "READ"},{"addresses": ["127.0.0.1:9004"], "role": "ROUTE"}]]
9+
SUCCESS {}
10+
C: RESET
11+
S: SUCCESS {}
12+
<EXIT>
Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,11 @@
1+
!: BOLT 3
2+
!: AUTO HELLO
3+
4+
C: RUN "CALL dbms.cluster.routing.getRoutingTable($context)" {"context": {}} {}
5+
PULL_ALL
6+
S: SUCCESS {"fields": ["ttl", "servers"]}
7+
RECORD [9223372036854775807, [{"addresses": ["127.0.0.1:9001"],"role": "WRITE"}, {"addresses": ["127.0.0.1:9002","127.0.0.1:9003"], "role": "READ"},{"addresses": ["127.0.0.1:9001","127.0.0.1:9002","127.0.0.1:9003"], "role": "ROUTE"}]]
8+
SUCCESS {}
9+
C: RESET
10+
S: SUCCESS {}
11+
<EXIT>

test/resources/boltstub/read_server_v3_read.script

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
!: BOLT 3
2+
!: AUTO HELLO
23
!: AUTO RESET
34

4-
C: HELLO {"scheme": "basic", "principal": "neo4j", "credentials": "password", "user_agent": "neo4j-javascript/0.0.0-dev"}
5-
S: SUCCESS {"server": "Neo4j/9.9.9", "connection_id": "bolt-123456789"}
65
C: RUN "MATCH (n) RETURN n.name" {} {"mode": "r"}
76
PULL_ALL
87
S: SUCCESS {"fields": ["n.name"]}

test/resources/boltstub/read_server_v3_read_tx.script

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,7 @@
11
!: BOLT 3
2+
!: AUTO HELLO
23
!: AUTO RESET
34

4-
C: HELLO {"scheme": "basic", "principal": "neo4j", "credentials": "password", "user_agent": "neo4j-javascript/0.0.0-dev"}
5-
S: SUCCESS {"server": "Neo4j/9.9.9", "connection_id": "bolt-123456789"}
65
C: BEGIN {"mode": "r"}
76
S: SUCCESS {}
87
C: RUN "MATCH (n) RETURN n.name" {} {"mode": "r"}

test/v1/driver.test.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -211,7 +211,7 @@ describe('driver', () => {
211211

212212
// Expect
213213
driver.onError = error => {
214-
expect(error.message).toEqual(`Server at localhost:7687 can't perform routing. Make sure you are connecting to a causal cluster`);
214+
expect(error.message).toContain('Could not perform discovery. No routing servers available.');
215215
expect(error.code).toEqual(neo4j.error.SERVICE_UNAVAILABLE);
216216
done();
217217
};

0 commit comments

Comments
 (0)