quic: refactor/improve error handling for busy event

jasnell · jasnell · commit cc89aac5f7d5 · 2020-07-09T07:14:07.000-07:00
Also, change setServerBusy into a setter PR-URL: #34247 Reviewed-By: Anna Henningsen <anna@addaleax.net>
diff --git a/doc/api/quic.md b/doc/api/quic.md
@@ -1330,8 +1330,8 @@ added: REPLACEME
 -->
 
 Emitted when the server busy state has been toggled using
-`quicSocket.setServerBusy()`. The callback is invoked with a single
-boolean argument indicating `true` if busy status is enabled,
+`quicSocket.serverBusy = true | false`. The callback is invoked with a
+single boolean argument indicating `true` if busy status is enabled,
 `false` otherwise. This event is strictly informational.
 
 ```js
@@ -1346,8 +1346,8 @@ socket.on('busy', (busy) => {
     console.log('Server is not busy');
 });
 
-socket.setServerBusy(true);
-socket.setServerBusy(false);
+socket.serverBusy = true;
+socket.serverBusy = false;
 ```
 
 This `'busy'` event may be emitted multiple times.
@@ -1874,6 +1874,18 @@ Set to `true` if the socket is not yet bound to the local UDP port.
 added: REPLACEME
 -->
 
+#### quicsocket.serverBusy
+<!-- YAML
+added: REPLACEME
+-->
+
+* Type: {boolean} When `true`, the `QuicSocket` will reject new connections.
+
+Setting `quicsocket.serverBusy` to `true` will tell the `QuicSocket`
+to reject all new incoming connection requests using the `SERVER_BUSY` QUIC
+error code. To begin receiving connections again, disable busy mode by setting
+`quicsocket.serverBusy = false`.
+
 #### quicsocket.serverBusyCount
 <!-- YAML
 added: REPLACEME
@@ -1911,19 +1923,6 @@ by artificially dropping received or transmitted packets.
 
 This method is *not* to be used in production applications.
 
-#### quicsocket.setServerBusy(\[on\])
-<!-- YAML
-added: REPLACEME
--->
-
-* `on` {boolean} When `true`, the `QuicSocket` will reject new connections.
-  **Defaults**: `true`.
-
-Calling `setServerBusy()` or `setServerBusy(true)` will tell the `QuicSocket`
-to reject all new incoming connection requests using the `SERVER_BUSY` QUIC
-error code. To begin receiving connections again, disable busy mode by calling
-`setServerBusy(false)`.
-
 #### quicsocket.statelessResetCount
 <!-- YAML
 added: REPLACEME
diff --git a/lib/internal/quic/core.js b/lib/internal/quic/core.js
@@ -851,7 +851,7 @@ class QuicSocket extends EventEmitter {
     highWaterMark: undefined,
     lookup: undefined,
     server: undefined,
-    serverBusy: undefined,
+    serverBusy: false,
     serverListening: false,
     serverSecureContext: undefined,
     sessions: new Set(),
@@ -1075,7 +1075,16 @@ class QuicSocket extends EventEmitter {
   // Called by the C++ internals to notify when server busy status is toggled.
   [kServerBusy](on) {
     this[kInternalState].serverBusy = on;
-    process.nextTick(emit.bind(this, 'busy', on));
+    // In a nextTick because the event ends up being
+    // emitted synchronously when quicSocket.serverBusy
+    // is called.
+    process.nextTick(() => {
+      try {
+        this.emit('busy', on);
+      } catch (error) {
+        this[kRejections](error, 'busy', on);
+      }
+    });
   }
 
   addEndpoint(options = {}) {
@@ -1490,10 +1499,10 @@ class QuicSocket extends EventEmitter {
 
   // Marking a server as busy will cause all new
   // connection attempts to fail with a SERVER_BUSY CONNECTION_CLOSE.
-  setServerBusy(on = true) {
+  set serverBusy(on) {
     const state = this[kInternalState];
     if (state.state === kSocketDestroyed)
-      throw new ERR_QUICSOCKET_DESTROYED('setServerBusy');
+      throw new ERR_QUICSOCKET_DESTROYED('serverBusy');
     validateBoolean(on, 'on');
     if (state.serverBusy !== on)
       this[kHandle].setServerBusy(on);
diff --git a/test/parallel/test-quic-server-busy-event-error-async.js b/test/parallel/test-quic-server-busy-event-error-async.js
@@ -0,0 +1,32 @@
+// Flags: --no-warnings
+'use strict';
+
+const common = require('../common');
+if (!common.hasQuic)
+  common.skip('missing quic');
+
+const assert = require('assert');
+const {
+  key,
+  cert,
+  ca,
+} = require('../common/quic');
+
+const { createQuicSocket } = require('net');
+
+const options = { key, cert, ca, alpn: 'zzz' };
+
+const server = createQuicSocket({ server: options });
+
+server.on('busy', common.mustCall(async () => {
+  throw new Error('boom');
+}));
+
+server.on('close', common.mustCall());
+
+server.on('error', common.mustCall((err) => {
+  assert.strictEqual(err.message, 'boom');
+}));
+
+assert.strictEqual(server.serverBusy, false);
+server.serverBusy = true;
diff --git a/test/parallel/test-quic-server-busy-event-error.js b/test/parallel/test-quic-server-busy-event-error.js
@@ -0,0 +1,32 @@
+// Flags: --no-warnings
+'use strict';
+
+const common = require('../common');
+if (!common.hasQuic)
+  common.skip('missing quic');
+
+const assert = require('assert');
+const {
+  key,
+  cert,
+  ca,
+} = require('../common/quic');
+
+const { createQuicSocket } = require('net');
+
+const options = { key, cert, ca, alpn: 'zzz' };
+
+const server = createQuicSocket({ server: options });
+
+server.on('busy', common.mustCall(() => {
+  throw new Error('boom');
+}));
+
+server.on('close', common.mustCall());
+
+server.on('error', common.mustCall((err) => {
+  assert.strictEqual(err.message, 'boom');
+}));
+
+assert.strictEqual(server.serverBusy, false);
+server.serverBusy = true;
