Repository navigation
Http2SecureServer Is Not Exported #21434
Description
Activity
What about using
.constructor?@ljharb I'm not sure how that gets me to
expect(server).toBeInstanceOf(http2.Http2SecureServer);
While
server.constructorrequires usingutil.inspectto get anything meaningful in a jest snapshot, andserver.constructor.namereturns"Http2SecureServer"but isn't the same as asserting the instance. Regardless, this still seems like a rather large omission and unnecessary pain point. I'd like to hear from the folks who worked on this to learn why it was left out of the exports.@shellscape Why not just use
http2.createSecureServer().constructorin your tests? Node.js won't export anything other thanHttp2SecureServerfromcreateSecureServer().This makes me wonder why the API is
http2.createSecureServer()and notnew http2.SecureServer().Reacted by Timothy Gu@targos It could easily be:
node/lib/internal/http2/core.js
Lines 2750 to 2762 in 8ab7ea6
/cc @jasnell
Requiring the api to be a constructor more tightly couples the implementation detail that it’s a
class.I think this keeps compatibility with how we do things in the other modules. It's easy enough to get the constructor instance for testing that IMO nothing needs to be changed.
Reacted by Jordan HarbandBut in other modules we expose the constructor.
http2is the outlier. It's also inconsistent with itself:http2.Http2ServerRequestandhttp2.Http2ServerResponseare exposed and the documentation mentionshttp2.Serverandhttp2.SecureServerwhich don't exist: https://nodejs.org/dist/latest-v10.x/docs/api/http2.html#http2_class_http2_http2serverrequestYeah the lack of consistency (as classes being exposed through exports are much more common) was why I initially created the issue. As @targos mentions, the way the documentation is written also suggests the class should be available on the export. There doesn't seem to be a cohesiveness between namespaces (net, tls, http, https, http2) as to how classes should or should not be exported. And no hint in the documentation as to which are and which are not.
But in other modules we expose the constructor.
And IMO it's a mistake which can lead to all sorts of repercussions later on. The current behaviour also makes it explicit that there's one API to create the server.
It's important to note that the only reason we exposed Http2ServerRequest and Http2ServerResponse is to allow users to create extended versions in an officially supported manner, where we provide a clear contract around their usage. (And by comparison, neither Http2Session nor Http2Stream are exposed.) That choice was also driven by the fact that http exported ServerRequest and ServerResponse. If that was not the case, we would be in a much better position to improve on it going forward.
I also don't see an applicable use case for exposing this particular class as one can easily get access to the constructor in order to do unit testing.
This design was intentional specifically to allow more flexibility in the implementation as it evolves. Exposing the constrictor directly exposes a number of issues in terms of maintenance. I'm -1 right now for exposing the constructor directly without having a clear use case
@jasnell @apapirovski I really don't anyone to gloss over the docs being unclear on this. If the constructor is not exposed, I can definitely live with that. But the docs at the very least need help in stating which are exported and which are not.
- added a commit that references this issue
on Aug 12, 2018 - added a commit that references this issue
on Oct 16, 2018 - added a commit that references this issue
on May 5, 2024 - added a commit that references this issue
on Apr 16, 2025
While writing tests for a project I found myself wanting to assert that the return value from a function was an instance of
Http2SecureServer. However, upon trying to find the export forHttp2SecureServer, I wound up at https://github.com/nodejs/node/blob/master/lib/internal/http2/core.js and found that the class isn't actually listed in the exports. So there's no discernible way to run aninstanceofagainst my return variable, as I can't reference the class.It's an unfortunate limitation; while testing that an object is an instance of
tls.Serveris fine and good, I would much rather add an extra layer of assertiveness and make sure that my return value was in fact an instance ofHttp2SecureServer, as the mechanisms in my use case can also returnhttps.Server, which also inherits fromtls.Server.On the surface this seems like a glaring omission. But there's probably a reason for that. What's the scoop?
Update: It would seem the same is true for
Http2Serveras well.