SocketCluster / socketcluster-client

JavaScript client for SocketCluster
MIT License
293 stars 91 forks source link

cid is not being sent to the server with socket.emit(..) #84

Closed Panoplos closed 7 years ago

Panoplos commented 7 years ago

I have the following code that I am using to wire up a network interface for apollo-client:

import socketCluster from 'socketcluster-client'

export default (options) => {
    console.log(`>>> Inside makeNetworkInterface() options => ${JSON.stringify(options)}`)
    return {
        query: async (request) => {
            console.log(`>>> Inside makeNetworkInterface.query() request => ${JSON.stringify(request)}`)
            const socket = socketCluster.connect(options)
            socket.emit('graphql', request)
            socket.on('graphql', data => Promise.resolve(data))
            socket.on('error', error => Promise.reject(error))
        }
    }
}

The server side is quite simple, too:

export default async (exchange, socket, debug) => {
    const schema = await rootSchema()
    return async (data, callback) => {
        const { query, variables } = data;
        const authToken = socket.getAuthToken();
        const context = {
            authToken,
            exchange,
            socket
        };
        // response = {errors, data}
        const result = await graphql(schema, query.loc.source.body, {}, context, variables);
        if (result.errors) {
            debug.error('DEBUG GraphQL Error:', result.errors);
        }
        const resolvedQueries = Object.keys(result.data);
        const firstQuery = resolvedQueries[0];
        const clientValue = blacklist.includes(firstQuery) ? true : result;
        callback(null, clientValue);
    }
}

Which is called by:

export default (exchange) => async (socket) => {
    debug.log(`Client connected >> ${socket.id}`)
    // It is necessary to await the creation of the handler, as it may create the schema.
    const graphQLHandler = await scGraphQLHandler(exchange, socket, debug)
    socket.on('graphql', graphQLHandler);
    socket.on('disconnect', () => {
        debug.log('Client disconnected <<', socket.id)
    })
}

When utilising this code, the server is not responding to the socket.emit(...). I traced into the the scsocket.js and response.js call stack to see what might be going wrong, and it appears that the Response.id is not getting set. With other message types, I can see that the cid is set to something, so the Response constructor sets its id appropriately, but for some reason my 'graphql' message does not have a cid. Here is an example of what is getting sent, pinched from scsocket.js `this.socket.on('message',...):

"{"event":"graphql","data":{"query":{"kind":"Document","definitions":[{"kind":"OperationDefinition","operation":"query","variableDefinitions":[],"directives":[],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","alias":null,"name":{"kind":"Name","value":"account"},"arguments":[{"kind":"Argument","name":{"kind":"Name","value":"email"},"value":{"kind":"StringValue","value":"test@testing.com"}}],"directives":[],"selectionSet":{"kind":"SelectionSet","selections":[{"kind":"Field","alias":null,"name":{"kind":"Name","value":"email"},"arguments":[],"directives":[],"selectionSet":null},{"kind":"Field","alias":null,"name":{"kind":"Name","value":"firstName"},"arguments":[],"directives":[],"selectionSet":null},{"kind":"Field","alias":null,"name":{"kind":"Name","value":"lastName"},"arguments":[],"directives":[],"selectionSet":null},{"kind":"Field","name":{"kind":"Name","value":"__typename"}}]}}]}}],"loc":{"start":0,"end":90,"source":{"body":"\n\tquery {\n\t\taccount(email: \"test@testing.com\") {\n\t\t\temail\n\t\t\tfirstName\n\t\t\tlastName\n\t\t}\n\t}\n","name":"GraphQL request","locationOffset":{"line":1,"column":1}}}},"operationName":null}}"

I didn't see anywhere in the docs that it is necessary for me to append a cid to my message, so there must be something wrong in my code. Any clarification will be greatly appreciated.

jondubois commented 7 years ago

@Panoplos The SC protocol doesn't require each event to have a cid - It is only necessary to provide one if the client expects a response from the server.

Currently, the socketcluster-client only generates a cid if the emit was given a callback as a third argument like this:

socket.emit('graphql', data, function (err, data) {
  // Handle response here.
});

Emitted events which don't have a response handler are not tracked by the client or server so that's why there is no need for a cid. You can provide your own application-level id as part of the data object if you like.

Is there a reason you want to use the cid specifically? We could add an option to the SC client to make it always send a cid...

Panoplos commented 7 years ago

Well, it is not a request for the cid as much as trying to figure out why the client is not receiving any response back. I am using socket.on('graphql', ...) after calling emit. Is this the wrong way to go about it?

Panoplos commented 7 years ago

OK, I see you are saying that a response from the server will be ignored server-side without a callback being specified on the client. Correct? I changed to a callback mechanism and now am getting a response. I based this code off of a youtube tutorial. Seems they had it wrong. The documentation uses callbacks, so I shall go that route.

jondubois commented 7 years ago

@Panoplos You can do it like you mentioned before, but then you'd have to explicitly invoke socket.emit('graphql', ...) from the server side too and it doesn't guarantee that it's a response to THAT specific emit. If you provide a callback then you can individually track multiple emits of the same event.

The docs for that are here: http://socketcluster.io/#!/docs/handling-failure

Panoplos commented 7 years ago

Thanks for the help. Closed.