Subscriptions-transport-ws: Suggestions to improve Promise detection

[malditogeek]

Hello!
I wanted to ask for some feedback before opening a PR. I identified an issue with the way subscriptions-transport-ws deals with Promises. In concrete this line right here: https://github.com/apollographql/subscriptions-transport-ws/blob/06ba1e8d6395089c4de92308bd353e4fc598f19d/src/server.ts#L373

The issue I found is that Google's Trace Agent wraps requests in a different implementation of Promise and promiseOrIterable instanceof Promise fails therefore throwing 'Invalidexecutereturn type! Only Promise or AsyncIterable are valid values!'

Do you have any suggestions about how to improve this? Promises are quiet tricky to detect and unfortunately lodash doesn't have a helper yet (https://github.com/lodash/lodash/pull/739)

graphql itself has its own function that check if the objects has a then function in it:

https://github.com/graphql/graphql-js/blob/1017befa7270cc0f074308056cea6115be498a99/src/execution/execute.js#L1256-L1266

Works for you? I can open a PR with that implementation if you feel is solid.

[dflor003]

I've been stuck for two days on the same issue! A fix for this would be much appreciated!

Any reason not to just check that the function is thenable? Perhaps instead of an instanceof check, something like this:

function isPromiseLike(obj: any) {
  return !!obj &&  typeof obj.then === 'function' && typeof obj.catch === 'function';
}

[dflor003]

I was able to verify that swapping out the instanceof check for the above check, does indeed fix my problem. PR to come soon.

[dflor003]

@malditogeek: I pushed up a branch to test these changes. If you want to try them out ahead of the PR going through, just add it:

via npm:

npm install --save https://github.com/dflor003/subscriptions-transport-ws.git#test-release

or via yarn:

yarn add https://github.com/dflor003/subscriptions-transport-ws.git#test-release

[seeden]

I think that this is related to https://github.com/nodejs/help/issues/557
I have same problem with PM2

process.binding('util').isPromise(value)

[NeoPhi]

Relaxed check for Promise merged in and released.