Skip to content

Commit 3e417f5

Browse files
mcollinaaduh95
andcommitted
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> (cherry picked from commit a631264)
1 parent b144b30 commit 3e417f5

10 files changed

Lines changed: 82 additions & 84 deletions

doc/api/deprecations.md

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

38643864
<!-- YAML
38653865
changes:
3866+
- version: v22.22.3
3867+
pr-url: https://github.com/nodejs/node/pull/58293
3868+
description: End-of-Life.
38663869
- version: v22.17.0
38673870
pr-url: https://github.com/nodejs/node/pull/58313
38683871
description: Documentation-only deprecation.
38693872
-->
38703873

3871-
Type: Documentation-only
3874+
Type: End-of-Life
38723875

3873-
The support for priority signaling has been deprecated in the [RFC 9113][], and
3874-
will be removed in future versions of Node.js.
3876+
The support for priority signaling has been removed following its deprecation in the [RFC 9113][].
38753877

38763878
### DEP0195: Instantiating `node:http` classes without `new`
38773879

doc/api/http2.md

Lines changed: 16 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -1071,6 +1071,10 @@ The `'origin'` event is only emitted when using a secure TLS connection.
10711071
<!-- YAML
10721072
added: v8.4.0
10731073
changes:
1074+
- version: v22.22.3
1075+
pr-url: https://github.com/nodejs/node/pull/58293
1076+
description: The `weight` option is now ignored, setting it will trigger a
1077+
runtime warning.
10741078
- version: v22.17.0
10751079
pr-url: https://github.com/nodejs/node/pull/58313
10761080
description: Following the deprecation of priority signaling as of RFC 9113,
@@ -1093,10 +1097,6 @@ changes:
10931097
**Default:** `false`.
10941098
* `parent` {number} Specifies the numeric identifier of a stream the newly
10951099
created stream is dependent on.
1096-
* `weight` {number} Specifies the relative dependency of a stream in relation
1097-
to other streams with the same `parent`. The value is a number between `1`
1098-
and `256` (inclusive). This has been **deprecated** in [RFC 9113][], and
1099-
support for it will be removed in future versions of Node.js.
11001100
* `waitForTrailers` {boolean} When `true`, the `Http2Stream` will emit the
11011101
`'wantTrailers'` event after the final `DATA` frame has been sent.
11021102
* `signal` {AbortSignal} An AbortSignal that may be used to abort an ongoing
@@ -1467,25 +1467,17 @@ numeric stream identifier.
14671467
<!-- YAML
14681468
added: v8.4.0
14691469
deprecated: v22.17.0
1470+
changes:
1471+
- version: v22.22.3
1472+
pr-url: https://github.com/nodejs/node/pull/58293
1473+
description: This method no longer sets the priority of the stream. Using it
1474+
now triggers a runtime warning.
14701475
-->
14711476

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

14901482
#### `http2stream.rstCode`
14911483

@@ -1582,6 +1574,10 @@ req.setTimeout(5000, () => req.close(NGHTTP2_CANCEL));
15821574
<!-- YAML
15831575
added: v8.4.0
15841576
changes:
1577+
- version: v22.22.3
1578+
pr-url: https://github.com/nodejs/node/pull/58293
1579+
description: The `state.weight` property is now always set to 16 and
1580+
`sumDependencyWeight` is always set to 0.
15851581
- version: v22.17.0
15861582
pr-url: https://github.com/nodejs/node/pull/58313
15871583
description: Following the deprecation of priority signaling as of RFC 9113,
@@ -1599,13 +1595,8 @@ Provides miscellaneous information about the current state of the
15991595
* `localClose` {number} `1` if this `Http2Stream` has been closed locally.
16001596
* `remoteClose` {number} `1` if this `Http2Stream` has been closed
16011597
remotely.
1602-
* `sumDependencyWeight` {number} The sum weight of all `Http2Stream`
1603-
instances that depend on this `Http2Stream` as specified using
1604-
`PRIORITY` frames. This has been **deprecated** in [RFC 9113][], and
1605-
support for it will be removed in future versions of Node.js.
1606-
* `weight` {number} The priority weight of this `Http2Stream`. This has been
1607-
**deprecated** in [RFC 9113][], and support for it will be removed in future
1608-
versions of Node.js.
1598+
* `sumDependencyWeight` {number} Legacy property, always set to `0`.
1599+
* `weight` {number} Legacy property, always set to `16`.
16091600

16101601
A current state of this `Http2Stream`.
16111602

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
promisify,
3131
SymbolAsyncDispose,
3232
SymbolDispose,
33+
deprecate,
34+
deprecateProperty,
3335
} = require('internal/util');
3436

3537
assertCrypto();
@@ -752,6 +754,11 @@ function onGoawayData(code, lastStreamID, buf) {
752754
}
753755
}
754756

757+
// TODO(aduh95): remove this in future semver-major
758+
const deprecateWeight = deprecateProperty('weight',
759+
'Priority signaling has been deprecated as of RFC 1993.',
760+
'DEP0194');
761+
755762
// When a ClientHttp2Session is first created, the socket may not yet be
756763
// connected. If request() is called during this time, the actual request
757764
// will be deferred until the socket is ready to go.
@@ -780,12 +787,14 @@ function requestOnConnect(headersList, options) {
780787
if (options.waitForTrailers)
781788
streamOptions |= STREAM_OPTION_GET_TRAILERS;
782789

790+
deprecateWeight(options);
791+
783792
// `ret` will be either the reserved stream ID (if positive)
784793
// or an error code (if negative)
785794
const ret = session[kHandle].request(headersList,
786795
streamOptions,
787796
options.parent | 0,
788-
options.weight | 0,
797+
NGHTTP2_DEFAULT_WEIGHT,
789798
!!options.exclusive);
790799

791800
// In an error condition, one of three possible response codes will be
@@ -830,11 +839,7 @@ function requestOnConnect(headersList, options) {
830839
//
831840
// Also sets the default priority options if they are not set.
832841
const setAndValidatePriorityOptions = hideStackFrames((options) => {
833-
if (options.weight === undefined) {
834-
options.weight = NGHTTP2_DEFAULT_WEIGHT;
835-
} else {
836-
validateNumber.withoutStackTrace(options.weight, 'options.weight');
837-
}
842+
deprecateWeight(options);
838843

839844
if (options.parent === undefined) {
840845
options.parent = 0;
@@ -890,25 +895,6 @@ function submitSettings(settings, callback) {
890895
}
891896
}
892897

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

2360-
priority(options) {
2361-
if (this.destroyed)
2362-
throw new ERR_HTTP2_INVALID_STREAM();
2363-
2364-
assertIsObject(options, 'options');
2365-
options = { ...options };
2366-
setAndValidatePriorityOptions(options);
2367-
2368-
const priorityFn = submitPriority.bind(this, options);
2369-
2370-
// If the handle has not yet been assigned, queue up the priority
2371-
// frame to be sent as soon as the ready event is emitted.
2372-
if (this.pending) {
2373-
this.once('ready', priorityFn);
2374-
return;
2375-
}
2376-
priorityFn();
2377-
}
2378-
23792346
sendTrailers(headers) {
23802347
if (this.destroyed || this.closed)
23812348
throw new ERR_HTTP2_INVALID_STREAM();
@@ -2548,6 +2515,12 @@ class Http2Stream extends Duplex {
25482515
}
25492516
}
25502517

2518+
// TODO(aduh95): remove this in future semver-major
2519+
Http2Stream.prototype.priority = deprecate(function priority(options) {
2520+
if (this.destroyed)
2521+
throw new ERR_HTTP2_INVALID_STREAM();
2522+
}, 'http2Stream.priority is longer supported after priority signalling was deprecated in RFC 1993', 'DEP0194');
2523+
25512524
function callTimeout(self, session) {
25522525
// If the session is destroyed, this should never actually be invoked,
25532526
// but just in case...

lib/internal/util.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,17 @@ function isPendingDeprecation() {
138138
!getOptionValue('--no-deprecation');
139139
}
140140

141+
function deprecateProperty(key, msg, code, isPendingDeprecation) {
142+
const emitDeprecationWarning = getDeprecationWarningEmitter(
143+
code, msg, undefined, false, isPendingDeprecation,
144+
);
145+
return (options) => {
146+
if (key in options) {
147+
emitDeprecationWarning();
148+
}
149+
};
150+
}
151+
141152
// Internal deprecator for pending --pending-deprecation. This can be invoked
142153
// at snapshot building time as the warning permission is only queried at
143154
// run time.
@@ -946,6 +957,8 @@ module.exports = {
946957
defineLazyProperties,
947958
defineReplaceableLazyAttribute,
948959
deprecate,
960+
deprecateInstantiation,
961+
deprecateProperty,
949962
emitExperimentalWarning,
950963
encodingsMap,
951964
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
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)