fix(subs): allow additional server variations (e.g., Tls, Http2)

[braineo]

fix #4198

Refer to discussion in the issue please.

[apollo-cla]

@braineo: Thank you for submitting a pull request! Before we can merge it, you'll need to sign the Apollo Contributor License Agreement here: https://contribute.apollographql.com/

[jedwards1211]

Seems like we should check for HttpsServer and Http2Server as well?

[braineo]

Hi @jedwards1211 I have not checked if https or http2 server are supported. Just reading from the function signature typing and seems it only take http.Server

https://github.com/apollographql/apollo-server/blob/master/packages/apollo-server-core/src/ApolloServer.ts#L595

import { Server as HttpServer } from 'http';
/// code
  public installSubscriptionHandlers(server: HttpServer | WebSocket.Server) 

[jedwards1211]

@abernix I'm guessing when using just JS it's possible to pass an https.Server or http2.Server? Maybe we should add those to the type defs?

[braineo]

@jedwards1211 Let me know what do you think. Should be easy to add.

[braineo]

@jedwards1211 @abernix

Ok I added type of http2 servers too. Just http2 module does not expose the class constructor that we cannot use isInstanceof

alternatively I used net.Server and tls.Server, what do you think?

As for the CI, it says

FAIL  packages/apollo-gateway/src/__tests__/integration/networkRequests.test.ts (5.838s)
  ● Downstream service health checks › Managed mode › Rolls over to new schema when health check succeeds

    : Timeout - Async callback was not invoked within the 5000ms timeout specified by jest.setTimeout.Timeout

I am not sure if it is a bug or just time out

[braineo]

@jedwards1211 @abernix

Seems the master branch is failing the same test and probably not introduced by this PR. Would you review or comment anything so that we can merge this PR soon please?

[jedwards1211]

?

[abernix]

Unintentional closing! Please stand-by: #4304

[jedwards1211]

gotcha, I see hump day decided to celebrate for you

[abernix]

I think this looks good, but can we write a number of test cases that exercise these types of servers with installSubscriptionHandlers?

[braineo]

@abernix where will be the best place to add?

inside packages/apollo-server-integration-testsuite/src/ApolloServer.ts and add more like this but ignore the httpServer pass down from a Promise?

        createApolloServer({
          typeDefs,
          resolvers,
        }).then(({ port, server, httpServer }) => {
          server.installSubscriptionHandlers(httpServer);

[braineo]

@abernix

I added test for websocket server.

[braineo]

@abernix Ping

[braineo]

@abernix Ping