Skip to content

Commit e40b358

Browse files
tegefaulkestegefaulkes
committed
wip: handling connections draining before closing socket
* Related #9 [ci skip]
1 parent 00e5404 commit e40b358

File tree

3 files changed

+74
-21
lines changed

3 files changed

+74
-21
lines changed

‎src/QUICClient.ts‎

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -91,7 +91,7 @@ class QUICClient extends EventTarget {
9191
const {
9292
p: errorP,
9393
rejectP: rejectErrorP
94-
} = utils.promise();
94+
} = utils.promise<never>();
9595
const handleQUICSocketError = (e: events.QUICSocketErrorEvent) => {
9696
rejectErrorP(e.detail);
9797
};
@@ -174,11 +174,25 @@ class QUICClient extends EventTarget {
174174
try {
175175
await Promise.race([connection.establishedP, errorP]);
176176
} catch (e) {
177+
logger.error(e.toString());
178+
// console.error(e);
179+
logger.debug(`Is shared?: ${isSocketShared}`);
180+
// Waiting for connection to destroy
181+
const destroyedProm = utils.promise<void>();
182+
connection.addEventListener('destroy', ()=> {
183+
destroyedProm.resolveP();
184+
},
185+
{
186+
once: true,
187+
},
188+
)
189+
await destroyedProm.p;
177190
if (!isSocketShared) {
178191
// Stop our own socket
179192
await socket.stop();
180193
}
181-
throw e;
194+
// Wrapping error to contain both stack traces
195+
throw new errors.ErrorQUICClient(e.message, {cause: e});
182196
}
183197

184198
// Remove the temporary socket error handler

‎src/QUICConnection.ts‎

Lines changed: 19 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -251,7 +251,7 @@ class QUICConnection extends EventTarget {
251251
}
252252

253253
/**
254-
* This provides the ability to destroy with a specific error
254+
* This provides the ability to destroy with a specific error. This will wait for the connection to fully drain.
255255
*/
256256
public async destroy(
257257
{
@@ -265,6 +265,11 @@ class QUICConnection extends EventTarget {
265265
} = {}
266266
) {
267267
this.logger.info(`Destroy ${this.constructor.name}`);
268+
const localError = this.conn.localError();
269+
const peerError = this.conn.peerError();
270+
if (localError != null) console.log(localError);
271+
if (peerError != null) console.log(peerError);
272+
console.log(this.conn.peerError());
268273
for (const stream of this.streamMap.values()) {
269274
await stream.destroy();
270275
}
@@ -288,15 +293,18 @@ class QUICConnection extends EventTarget {
288293
utils.never();
289294
}
290295
}
296+
console.log(this.conn.isClosed(), this.conn.isDraining());
291297
// If it is not closed, it could still be draining
292-
if (!this.conn.isClosed()) {
298+
if (this.conn.isDraining()) {
293299
// The `recv`, `send`, `timeout`, and `on_timeout` should continue to be called
294300
const { p: closeP, resolveP: resolveCloseP } = utils.promise();
295301
this.resolveCloseP = resolveCloseP;
302+
// FIXME: we need to process `send` AND `recv` until the connection is closed.
296303
await Promise.all([
297304
this.send(),
298305
closeP
299306
]);
307+
console.log('asd')
300308
}
301309
this.connectionMap.delete(this.connectionId);
302310
this.dispatchEvent(new events.QUICConnectionDestroyEvent());
@@ -339,6 +347,9 @@ class QUICConnection extends EventTarget {
339347
this.logger.debug(`Did a recv ${data.byteLength}`);
340348
this.conn.recv(data, recvInfo);
341349
} catch (e) {
350+
this.logger.error(e.toString());
351+
// console.error(e);
352+
// console.log(this.conn.isClosed());
342353
// Depending on the exception, the `this.conn.recv`
343354
// may have automatically started closing the connection
344355
this.dispatchEvent(
@@ -427,13 +438,12 @@ class QUICConnection extends EventTarget {
427438

428439
try {
429440
if (this.conn.isClosed()) {
430-
431-
432441
if (this.resolveCloseP != null) this.resolveCloseP();
433442
return;
434-
} else if (this.conn.isDraining()) {
435-
return;
436443
}
444+
// else if (this.conn.isDraining()) {
445+
// return;
446+
// }
437447
const sendBuffer = new Uint8Array(quiche.MAX_DATAGRAM_SIZE);
438448
let sendLength: number;
439449
let sendInfo: SendInfo;
@@ -481,7 +491,7 @@ class QUICConnection extends EventTarget {
481491
sendInfo.to.host
482492
);
483493
} catch (e) {
484-
console.error(e);
494+
this.logger.error(e.toString())
485495
this.dispatchEvent(new events.QUICConnectionErrorEvent({ detail: e }));
486496
return;
487497
}
@@ -491,11 +501,12 @@ class QUICConnection extends EventTarget {
491501
this.logger.debug('SEND FINALLY');
492502

493503
this.setTimeout();
504+
this.logger.debug(`connection is closed?: ${this.conn.isClosed()}`)
505+
this.logger.debug(`connection is draining?: ${this.conn.isDraining()}`)
494506
if (
495507
this[status] !== 'destroying' &&
496508
(this.conn.isClosed() || this.conn.isDraining())
497509
) {
498-
499510
this.logger.debug('CALLING DESTROY');
500511

501512
await this.destroy();

‎tests/QUICClient.test.ts‎

Lines changed: 39 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { fc } from '@fast-check/jest';
1010
import * as tlsUtils from './tlsUtils';
1111
import * as certFixtures from './fixtures/certFixtures';
1212
import { promise } from "@/utils";
13+
import { sleep } from './utils';
1314

1415
const tlsArb = fc.oneof(
1516
certFixtures.tlsConfigExampleArb,
@@ -69,7 +70,8 @@ describe(QUICClient.name, () => {
6970
crypto,
7071
logger: logger.getChild(QUICServer.name),
7172
config: {
72-
tlsConfig
73+
tlsConfig,
74+
verifyPeer: false,
7375
}
7476
});
7577
server.addEventListener('connection', handleConnectionEventP);
@@ -82,6 +84,9 @@ describe(QUICClient.name, () => {
8284
localHost: '::' as Host,
8385
crypto,
8486
logger: logger.getChild(QUICClient.name),
87+
config: {
88+
verifyPeer: false,
89+
}
8590
});
8691
const conn = (await connectionEventP).detail;
8792
expect(conn.localHost).toBe('127.0.0.1');
@@ -96,7 +101,8 @@ describe(QUICClient.name, () => {
96101
crypto,
97102
logger: logger.getChild(QUICServer.name),
98103
config: {
99-
tlsConfig
104+
tlsConfig,
105+
verifyPeer: false,
100106
}
101107
});
102108
server.addEventListener('connection', handleConnectionEventP);
@@ -109,7 +115,10 @@ describe(QUICClient.name, () => {
109115
port: server.port,
110116
localHost: '::' as Host,
111117
crypto,
112-
logger: logger.getChild(QUICClient.name)
118+
logger: logger.getChild(QUICClient.name),
119+
config: {
120+
verifyPeer: false,
121+
}
113122
});
114123
const conn = (await connectionEventP).detail;
115124
expect(conn.localHost).toBe('::1');
@@ -124,7 +133,8 @@ describe(QUICClient.name, () => {
124133
crypto,
125134
logger: logger.getChild(QUICServer.name),
126135
config: {
127-
tlsConfig
136+
tlsConfig,
137+
verifyPeer: false,
128138
}
129139
});
130140
server.addEventListener('connection', handleConnectionEventP);
@@ -137,7 +147,10 @@ describe(QUICClient.name, () => {
137147
port: server.port,
138148
localHost: '::' as Host,
139149
crypto,
140-
logger: logger.getChild(QUICClient.name)
150+
logger: logger.getChild(QUICClient.name),
151+
config: {
152+
verifyPeer: false,
153+
}
141154
});
142155
const conn = (await connectionEventP).detail;
143156
expect(conn.localHost).toBe('::');
@@ -158,6 +171,7 @@ describe(QUICClient.name, () => {
158171
logger: logger.getChild(QUICClient.name),
159172
config: {
160173
maxIdleTimeout: 1000,
174+
verifyPeer: false,
161175
}
162176
})).rejects.toThrow(errors.ErrorQUICConnectionTimeout);
163177
});
@@ -182,7 +196,8 @@ describe(QUICClient.name, () => {
182196
crypto,
183197
logger: logger.getChild(QUICServer.name),
184198
config: {
185-
tlsConfig: certFixtures.tlsConfigFileRSA1
199+
tlsConfig: certFixtures.tlsConfigFileRSA1,
200+
verifyPeer: false,
186201
}
187202
});
188203
server.addEventListener('connection', handleConnectionEventP);
@@ -195,6 +210,9 @@ describe(QUICClient.name, () => {
195210
localHost: '::' as Host,
196211
crypto,
197212
logger: logger.getChild(QUICClient.name),
213+
config: {
214+
verifyPeer: false,
215+
}
198216
});
199217
const peerCertChainInitial = client1.connection.conn.peerCertChain()
200218
server.updateConfig({
@@ -211,7 +229,8 @@ describe(QUICClient.name, () => {
211229
crypto,
212230
logger: logger.getChild(QUICServer.name),
213231
config: {
214-
tlsConfig: certFixtures.tlsConfigFileRSA1
232+
tlsConfig: certFixtures.tlsConfigFileRSA1,
233+
verifyPeer: false,
215234
}
216235
});
217236
server.addEventListener('connection', handleConnectionEventP);
@@ -224,6 +243,9 @@ describe(QUICClient.name, () => {
224243
localHost: '::' as Host,
225244
crypto,
226245
logger: logger.getChild(QUICClient.name),
246+
config: {
247+
verifyPeer: false,
248+
}
227249
});
228250
const peerCertChainInitial = client1.connection.conn.peerCertChain()
229251
server.updateConfig({
@@ -236,6 +258,9 @@ describe(QUICClient.name, () => {
236258
localHost: '::' as Host,
237259
crypto,
238260
logger: logger.getChild(QUICClient.name),
261+
config: {
262+
verifyPeer: false,
263+
}
239264
});
240265
const peerCertChainNew = client2.connection.conn.peerCertChain()
241266
expect(peerCertChainNew![0].toString()).not.toStrictEqual(peerCertChainInitial![0].toString());
@@ -250,15 +275,17 @@ describe(QUICClient.name, () => {
250275
crypto,
251276
logger: logger.getChild(QUICServer.name),
252277
config: {
253-
tlsConfig: certFixtures.tlsConfigFileRSA1,
254-
verifyPeer: true,
255-
verifyFromPemFile: certFixtures.tlsConfigFileRSA2.certChainFromPemFile
278+
tlsConfig: certFixtures.tlsConfigFileRSA2,
279+
verifyPeer: false,
280+
verifyFromPemFile: certFixtures.tlsConfigFileRSA2.certChainFromPemFile,
281+
logKeys: "tmp/key.log",
256282
}
257283
});
258284
const handleConnectionEventProm = promise<any>()
259285
server.addEventListener('connection', handleConnectionEventProm.resolveP);
260286
await server.start({
261287
host: '127.0.0.1' as Host,
288+
port: 55555 as Port,
262289
});
263290
// Connection should succeed
264291
const client = await QUICClient.createQUICClient({
@@ -268,11 +295,12 @@ describe(QUICClient.name, () => {
268295
crypto,
269296
logger: logger.getChild(QUICClient.name),
270297
config: {
271-
verifyPeer: false,
298+
verifyPeer: true,
272299
tlsConfig: certFixtures.tlsConfigFileRSA2,
273300
verifyFromPemFile: certFixtures.tlsConfigFileRSA2.certChainFromPemFile
274301
}
275302
});
303+
console.log('wait for connection');
276304
await handleConnectionEventProm.p
277305
await client.destroy();
278306
await server.stop();

0 commit comments

Comments
 (0)