Open zeuscronos opened 2 months ago
The way the withFilter method is typed makes inference impossible, which pothos can't fix for you.
I would probably just not use it, and do something like this instead:
builder.subscriptionField('messageEvent', (t: AppSubscriptionFieldBuilder) =>
t.field({
type: GqlMessageEvent,
args: {
channel: t.arg.string({ required: true }),
},
subscribe: async function*(_, { channel }, context) {
const iter = context.pubsub.asyncIterator([NOTIFICATION_CHANNEL_MESSAGE]);
for await (const payload of iter) {
if ((payload.messageEvent.channel === channel) && (!(await shouldIgnoreMessageEvents))) {
yield payload;
}
}
},
resolve: ({ messageEvent }) => messageEvent
})
I didn't test this, but hopefully that will get you going in the right direction
That was the code I initially had but even though it works as expected I had troubles when doing unit tests involving multiple cycles of subscriptions/desubscriptions. I got this warning...
(node:12788) MaxListenersExceededWarning: Possible EventEmitter memory leak detected. 11 CHANNEL_MESSAGE listeners added to [EventEmitter]. Use emitter.setMaxListeners() to increase limit (Use
node --trace-warnings ...
to show where the warning was created)
... which is a clear concern for me because I'm afraid of leaving event listeners around in the server corresponding to subscriptions that already finished.
Alternatively you can do this other simple experiment with your proposed code as indicated in the screenshot below, by doing 11 cycles of Subscriptions/Desubscriptions...
... then you will get the warning I mentioned above.
Looks like with that code, if you don't yield the event (because the condition), then even though if the subscription is finished the event listener gets stuck around causing possible memory leaks which is something we don't want.
However, when you use withFilter(...)
you don't get that warning. Then I'm assuming that the logic inside: withFilter(...)
handles better the situation when the event is not yielded.
I can reproduce the issue you are talking about, but I think this is a bug outside the scope of Pothos. I did notice that I wasn't seeing the same warnings when using yoga instead of apollo.
Pothos just passes the resolve and subscribe functions into the generated schema. I think the place to fix this would be for graphql-subscriptions to improve the types for their withFilter function, so that it can be used properly, but there may also be something interesting happening inside of apollo.
I added some additional logging and what I was seeing was that wrapping the for loop in a try/finally block, it didn't appear to be triggering the return
on the async iterator, which is what probably clears up listeners.
If you have anymore insight into whats going on, I'm definitely interested, but I don't think this is something that can be fixed inside pothos, since pothos doesn't really control how these iterators are invoked by the server
Thank you @hayes. I think in the future we could open a thread on Apollo repo pointing to this issue I mentioned here so maybe with those both cross references we could optimize the combination of Apollo + Pothos.
I have this very simple dummy project using:
Pothos
. You can download it from here.On file:
/src/graphql/schema.ts
I have the following simple code:My Problem: Looks like Pothos and
withFilter
function are not working well together. Then I'm having the 2 issues below:Issue 1: For the subscription field:
messageEvent
I'm getting these 2 TypeScript errors:Typescript Error 1: Type 'ResolverFn' is not assignable to type 'Subscriber<{}, InputShapeFromFields<{ channel: InputFieldRef<string, "Arg">; }>, any, unknown>'. Type 'AsyncIterator<any, any, undefined>' is not assignable to type 'MaybePromise<AsyncIterable>'.
Typescript Error 2: Type '({ messageEvent }: { messageEvent: any; }) => any' is not assignable to type 'Resolver<unknown, InputShapeFromFields<{ channel: InputFieldRef<string, "Arg">; }>, any, { channel: string; message: string; }, unknown>'. Types of parameters '__0' and 'parent' are incompatible. Type 'unknown' is not assignable to type '{ messageEvent: any; }'.
Issue 2: The
channel
argument in the second parameter ofwithFilter
(which is a function) is not actually recognized as an argument when just above I used:channel: t.arg.string({ required: true })
. However if I get rid of:withFilter
and just use the code below, thenchannel
is recognized as an argument with type:string
: