8000 http2: remove support for priority signaling · nodejs/node@0b987e5 · GitHub
[go: up one dir, main page]
Skip to content

Commit 0b987e5

Browse files
mcollinaaduh95
mcollina
and
aduh95
committed
http2: remove support for priority signaling
Signed-off-by: Matteo Collina <hello@matteocollina.com> Co-authored-by: Antoine du Hamel <duhamelantoine1995@gmail.com> Refs: https://datatracker.ietf.org/doc/html/rfc9113#section-5.3.1 PR-URL: #58293 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Darshan Sen <raisinten@gmail.com> Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Tim Perry <pimterry@gmail.com> Reviewed-By: Filip Skokan <panva.ip@gmail.com> Reviewed-By: Michael Dawson <midawson@redhat.com> Reviewed-By: Rafael Gonzaga <rafael.nunu@hotmail.com> Reviewed-By: Benjamin Gruenbaum <benjamingr@gmail.com>
1 parent d0b8959 commit 0b987e5

10 files changed

+81
-84
lines changed

‎doc/api/deprecations.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -3933,15 +3933,17 @@ an internal nodejs implementation rather than a public facing API, use `node:str
39333933

39343934
<!-- YAML
39353935
changes:
3936+
- version: REPLACEME
3937+
pr-url: https://github.com/nodejs/node/pull/58293
3938+
description: End-of-Life.
39363939
- version: REPLACEME
39373940
pr-url: https://github.com/nodejs/node/pull/58313
39383941
description: Documentation-only deprecation.
39393942
-->
39403943

3941-
Type: Documentation-only
3944+
Type: End-of-Life
39423945

3943-
The support for priority signaling has been deprecated in the [RFC 9113][], and
3944-
will be removed in future versions of Node.js.
3946+
The support for priority signaling has been removed following its deprecation in the [RFC 9113][].
39453947

39463948
### DEP0195: Instantiating `node:http` classes without `new`
39473949

‎doc/api/http2.md‎

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1072,6 +1072,10 @@ The `'origin'` event is only emitted when using a secure TLS connection.
10721072
<!-- YAML
10731073
added: v8.4.0
10741074
changes:
1075+
- version: REPLACEME
1076+
pr-url: https://github.com/nodejs/node/pull/58293
1077+
description: The `weight` option is now ignored, setting it will trigger a
1078+
runtime warning.
10751079
- version: REPLACEME
10761080
pr-url: https://github.com/nodejs/node/pull/58313
10771081
description: Following the deprecation of priority signaling as of RFC 1993,
@@ -1090,10 +1094,6 @@ changes:
10901094
**Default:** `false`.
10911095
* `parent` {number} Specifies the numeric identifier of a stream the newly
10921096
created stream is dependent on.
1093-
* `weight` {number} Specifies the relative dependency of a stream in relation
1094-
to other streams with the same `parent`. The value is a number between `1`
1095-
and `256` (inclusive). This has been **deprecated** in [RFC 9113][], and
1096-
support for it will be removed in future versions of Node.js.
10971097
* `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
10981098
`'wantTrailers'` event after the final `DATA` frame has been sent.
10991099
* `signal` {AbortSignal} An AbortSignal that may be used to abort an ongoing
@@ -1464,25 +1464,17 @@ numeric stream identifier.
14641464
<!-- YAML
14651465
added: v8.4.0
14661466
deprecated: REPLACEME
1467+
changes:
1468+
- version: REPLACEME
1469+
pr-url: https://github.com/nodejs/node/pull/58293
1470+
description: This method no longer sets the priority of the stream. Using it
1471+
now triggers a runtime warning.
14671472
-->
14681473

14691474
> Stability: 0 - Deprecated: support for priority signaling has been deprecated
14701475
> in the [RFC 9113][] and is no longer supported in Node.js.
14711476
1472-
* `options` {Object}
1473-
* `exclusive` {boolean} When `true` and `parent` identifies a parent Stream,
1474-
this stream is made the sole direct dependency of the parent, with
1475-
all other existing dependents made a dependent of this stream. **Default:**
1476-
`false`.
1477-
* `parent` {number} Specifies the numeric identifier of a stream this stream
1478-
is dependent on.
1479-
* `weight` {number} Specifies the relative dependency of a stream in relation
1480-
to other streams with the same `parent`. The value is a number between `1`
1481-
and `256` (inclusive).
1482-
* `silent` {boolean} When `true`, changes the priority locally without
1483-
sending a `PRIORITY` frame to the connected peer.
1484-
1485-
Updates the priority for this `Http2Stream` instance.
1477+
Empty method, only there to maintain some backward compatibility.
14861478

14871479
#### `http2stream.rstCode`
14881480

@@ -1579,6 +1571,10 @@ req.setTimeout(5000, () => req.close(NGHTTP2_CANCEL));
15791571
<!-- YAML
15801572
added: v8.4.0
15811573
changes:
1574+
- version: REPLACEME
1575+
pr-url: https://github.com/nodejs/node/pull/58293
1576+
description: The `state.weight` property is now always set to 16 and
1577+
`sumDependencyWeight` is always set to 0.
15821578
- version: REPLACEME
15831579
pr-url: https://github.com/nodejs/node/pull/58313
15841580
description: Following the deprecation of priority signaling as of RFC 1993,
@@ -1596,13 +1592,8 @@ Provides miscellaneous information about the current state of the
15961592
* `localClose` {number} `1` if this `Http2Stream` has been closed locally.
15971593
* `remoteClose` {number} `1` if this `Http2Stream` has been closed
15981594
remotely.
1599-
* `sumDependencyWeight` {number} The sum weight of all `Http2Stream`
1600-
instances that depend on this `Http2Stream` as specified using
1601-
`PRIORITY` frames. This has been **deprecated** in [RFC 9113][], and
1602-
support for it will be removed in future versions of Node.js.
1603-
* `weight` {number} The priority weight of this `Http2Stream`. This has been
1604-
**deprecated** in [RFC 9113][], and support for it will be removed in future
1605-
versions of Node.js.
1595+
* `sumDependencyWeight` {number} Legacy property, always set to `0`.
1596+
* `weight` {number} Legacy property, always set to `16`.
16061597

16071598
A current state of this `Http2Stream`.
16081599

‎lib/internal/http2/core.js‎

Lines changed: 17 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,8 @@ const {
3030
customInspectSymbol: kInspect,
3131
kEmptyObject,
3232
promisify,
33+
deprecate,
34+
deprecateProperty,
3335
} = require('internal/util');
3436

3537
assertCrypto();
@@ -748,6 +750,11 @@ function onGoawayData(cod 5DA8 e, lastStreamID, buf) {
748750
}
749751
}
750752

753+
// TODO(aduh95): remove this in future semver-major
754+
const deprecateWeight = deprecateProperty('weight',
755+
'Priority signaling has been deprecated as of RFC 1993.',
756+
'DEP0194');
757+
751758
// When a ClientHttp2Session is first created, the socket may not yet be
752759
// connected. If request() is called during this time, the actual request
753760
// will be deferred until the socket is ready to go.
@@ -776,12 +783,14 @@ function requestOnConnect(headersList, headersParam, options) {
776783
if (options.waitForTrailers)
777784
streamOptions |= STREAM_OPTION_GET_TRAILERS;
778785

786+
deprecateWeight(options);
787+
779788
// `ret` will be either the reserved stream ID (if positive)
780789
// or an error code (if negative)
781790
const ret = session[kHandle].request(headersList,
782791
streamOptions,
783792
options.parent | 0,
784-
options.weight | 0,
793+
NGHTTP2_DEFAULT_WEIGHT,
785794
!!options.exclusive);
786795

787796
// In an error condition, one of three possible response codes will be
@@ -826,11 +835,7 @@ function requestOnConnect(headersList, headersParam, options) {
826835
//
827836
// Also sets the default priority options if they are not set.
828837
const setAndValidatePriorityOptions = hideStackFrames((options) => {
829-
if (options.weight === undefined) {
830-
options.weight = NGHTTP2_DEFAULT_WEIGHT;
831-
} else {
832-
validateNumber.withoutStackTrace(options.weight, 'options.weight');
833-
}
838+
deprecateWeight(options);
834839

835840
if (options.parent === undefined) {
836841
options.parent = 0;
@@ -886,25 +891,6 @@ function submitSettings(settings, callback) {
886891
}
887892
}
888893

889-
// Submits a PRIORITY frame to be sent to the remote peer
890-
// Note: If the silent option is true, the change will be made
891-
// locally with no PRIORITY frame sent.
892-
function submitPriority(options) {
893-
if (this.destroyed)
894-
return;
895-
this[kUpdateTimer]();
896-
897-
// If the parent is the id, do nothing because a
898-
// stream cannot be made to depend on itself.
899-
if (options.parent === this[kID])
900-
return;
901-
902-
this[kHandle].priority(options.parent | 0,
903-
options.weight | 0,
904-
!!options.exclusive,
905-
!!options.silent);
906-
}
907-
908894
// Submit a GOAWAY frame to be sent to the remote peer.
909895
// If the lastStreamID is set to <= 0, then the lastProcStreamID will
910896
// be used. The opaqueData must either be a typed array or undefined
@@ -2314,25 +2300,6 @@ class Http2Stream extends Duplex {
23142300
}
23152301
}
23162302

2317-
priority(options) {
2318-
if (this.destroyed)
2319-
throw new ERR_HTTP2_INVALID_STREAM();
2320-
2321-
assertIsObject(options, 'options');
2322-
options = { ...options };
2323-
setAndValidatePriorityOptions(options);
2324-
2325-
const priorityFn = submitPriority.bind(this, options);
2326-
2327-
// If the handle has not yet been assigned, queue up the priority
2328-
// frame to be sent as soon as the ready event is emitted.
2329-
if (this.pending) {
2330-
this.once('ready', priorityFn);
2331-
return;
2332-
}
2333-
priorityFn();
2334-
}
2335-
23362303
sendTrailers(headers) {
23372304
if (this.destroyed || this.closed)
23382305
throw new ERR_HTTP2_INVALID_STREAM();
@@ -2505,6 +2472,12 @@ class Http2Stream extends Duplex {
25052472
}
25062473
}
25072474

2475+
// TODO(aduh95): remove this in future semver-major
2476+
Http2Stream.prototype.priority = deprecate(function priority(options) {
2477+
if (this.destroyed)
2478+
throw new ERR_HTTP2_INVALID_STREAM();
2479+
}, 'http2Stream.priority is longer supported after priority signalling was deprecated in RFC 1993', 'DEP0194');
2480+
25082481
function callTimeout(self, session) {
25092482
// If the session is destroyed, this should never actually be invoked,
25102483
// but just in case...

‎lib/internal/util.js‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -135,6 +135,17 @@ function isPendingDeprecation() {
135135
!getOptionValue('--no-deprecation');
136136
}
137137

138+
function deprecateProperty(key, msg, code, isPendingDeprecation) {
139+
const emitDeprecationWarning = getDeprecationWarningEmitter(
140+
code, msg, undefined, false, isPendingDeprecation,
141+
);
142+
return (options) => {
143+
if (key in options) {
144+
emitDeprecationWarning();
145+
}
146+
};
147+
}
148+
138149
// Internal deprecator for pending --pending-deprecation. This can be invoked
139150
// at snapshot building time as the warning permission is only queried at
140151
// run time.
@@ -947,6 +958,7 @@ module.exports = {
947958
defineReplaceableLazyAttribute,
948959
deprecate,
949960
deprecateInstantiation,
961+
deprecateProperty,
950962
emitExperimentalWarning,
951963
encodingsMap,
952964
exposeInterface,

‎test/parallel/test-http2-client-priority-before-connect.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,11 @@ if (!common.hasCrypto)
55
common.skip('missing crypto');
66
const h2 = require('http2');
77

8+
common.expectWarning(
9+
'DeprecationWarning',
10+
'http2Stream.priority is longer supported after priority signalling was deprecated in RFC 1993',
11+
'DEP0194');
12+
813
const server = h2.createServer();
914

1015
// We use the lower-level API here

‎test/parallel/test-http2-client-request-options-errors.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ const http2 = require('http2');
1111

1212
const optionsToTest = {
1313
endStream: 'boolean',
14-
weight: 'number',
1514
parent: 'number',
1615
exclusive: 'boolean',
1716
silent: 'boolean'

‎test/parallel/test-http2-client-set-priority.js‎

Lines changed: 7 additions & 1 deletion
< C8D0 tr class="diff-line-row">
Original file line numberDiff line numberDiff line change
@@ -6,10 +6,16 @@ if (!common.hasCrypto)
66
const assert = require('assert');
77
const http2 = require('http2');
88

9+
common.expectWarning(
10+
'DeprecationWarning',
11+
'Priority signaling has been deprecated as of RFC 1993.',
12+
'DEP0194');
13+
914
const checkWeight = (actual, expect) => {
1015
const server = http2.createServer();
1116
server.on('stream', common.mustCall((stream, headers, flags) => {
12-
assert.strictEqual(stream.state.weight, expect);
17+
assert.strictEqual(stream.state.sumDependencyWeight, 0);
18+
assert.strictEqual(stream.state.weight, 16);
1319
stream.respond();
1420
stream.end('test');
1521
}));

‎test/parallel/test-http2-priority-cycle-.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,11 @@ const assert = require('assert');
77
const http2 = require('http2');
88
const Countdown = require('../common/countdown');
99

10+
common.expectWarning(
11+
'DeprecationWarning',
12+
'http2Stream.priority is longer supported after priority signalling was deprecated in RFC 1993',
13+
'DEP0194');
14+
1015
const server = http2.createServer();
1116
const largeBuffer = Buffer.alloc(1e4);
1217

‎test/parallel/test-http2-priority-event.js‎

Lines changed: 9 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -3,21 +3,18 @@
33
const common = require('../common');
44
if (!common.hasCrypto)
55
common.skip('missing crypto');
6-
const assert = require('assert');
76
const h2 = require('http2');
87

8+
common.expectWarning(
9+
'DeprecationWarning',
10+
'http2Stream.priority is longer supported after priority signalling was deprecated in RFC 1993',
11+
'DEP0194');
12+
913
const server = h2.createServer();
1014

1115
// We use the lower-level API here
1216
server.on('stream', common.mustCall(onStream));
1317

14-
function onPriority(stream, parent, weight, exclusive) {
15-
assert.strictEqual(stream, 1);
16-
assert.strictEqual(parent, 0);
17-
assert.strictEqual(weight, 1);
18-
assert.strictEqual(exclusive, false);
19-
}
20-
2118
function onStream(stream, headers, flags) {
2219
stream.priority({
2320
parent: 0,
@@ -33,7 +30,7 @@ function onStream(stream, headers, flags) {
3330

3431
server.listen(0);
3532

36-
server.on('priority', common.mustCall(onPriority));
33+
server.on('priority', common.mustNotCall());
3734

3835
server.on('listening', common.mustCall(() => {
3936

@@ -48,7 +45,9 @@ server.on('listening', common.mustCall(() => {
4845
});
4946
});
5047

51-
req.on('priority', common.mustCall(onPriority));
48+
// The priority event is not supported anymore by nghttp2
49+
// since 1.65.0.
50+
req.on('priority', common.mustNotCall());
5251

5352
req.on('response', common.mustCall());
5453
req.resume();

‎test/parallel/test-http2-server-stream-session-destroy.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,11 @@ if (!common.hasCrypto)
66
const assert = require('assert');
77
const h2 = require('http2');
88

9+
common.expectWarning(
10+
'DeprecationWarning',
11+
'http2Stream.priority is longer supported after priority signalling was deprecated in RFC 1993',
12+
'DEP0194');
13+
914
const server = h2.createServer();
1015

1116
server.on('stream', common.mustCall((stream) => {

0 commit comments

Comments
 (0)
0