Don't fall back to Apollo when not in subprotocols

[jaylett]

If the server doesn't return Sec-WebSocket-Protocol, we can't default to graphql-ws (ie Apollo) if the transport was configured without it as a subprotocol. In that situation, default to graphql-transport-ws (GRAPHQLWS).

[jaylett]

I've encountered a live server that doesn't seem to include Sec-WebSocket-Protocol during initialisation, and speaks graphql-transport-ws not graphql-ws. (At least, carrying this patch locally makes it possible to talk to the server.)

I'm not sure how to write a test for this, because I don't really understand how the (mocked?) servers work.

[jaylett]

Hmm, the lint failure suggests that something other than the normal control path can create the adapter, given the normal WebsocketsProtocolTransportBase always sets subprotocols on the adapter. I'll push a fix for this once the other tests have run.

[leszekhanusz]

For the tests, you have different server pytest fixtures in the conftest.py file

server for the old apollo server and graphqlws_server for the graphql-ws protocol.
If you check those fixtures, you notice that only graphqlws_server sets the protocol back in a header.

Then you have fixtures like client_and_server and client_and_graphqlws_server which are just fixtures made to avoid duplicating the creation of the client and transport in each test.

We have the test_websocket_*.py files using server and client_and_server fixtures
We have the test_graphqlws_*.py files using graphqlws_server and client_and_graphqlws_server fixtures

So, if we want to test that your modification is working, you could in the test_graphqlws_subscription.py file create a test similar to test_graphqlws_subscription, but using the server fixture instead of the graphqlws_server fixture, which will not return the subprotocol and creating the client and session yourself instead of using the client_and_graphqlws_server fixture (see below the test test_graphqlws_subscription_sync for inspiration as it uses the graphqlws_server fixture directly)

[codecov]

Codecov Report

❌ Patch coverage is 80.00000% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.96%. Comparing base (7fb869a) to head (ed18026).
⚠️ Report is 51 commits behind head on master.

Files with missing lines	Patch %	Lines
gql/transport/websockets_protocol.py	80.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff             @@
##            master     #586      +/-   ##
===========================================
- Coverage   100.00%   99.96%   -0.04%     
===========================================
  Files           38       40       +2     
  Lines         2908     3314     +406     
===========================================
+ Hits          2908     3313     +405     
- Misses           0        1       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

[leszekhanusz]

I can add that test if you want.