graphql-platform
graphql-platform copied to clipboard
Subscriptions: subscriber and resolver parameters mismatch fails silently
Is there an existing issue for this?
- [X] I have searched the existing issues
Describe the bug
This example
public async IAsyncEnumerable<Item> SubItems(long id, [Service] IEventTarget eventTarget) {
await foreach(Event<Item> @event in eventTarget.PullEvents<Event<Item>>()) {
if (@event.Entity.Id != id) continue;
yield return @event.Entitiy;
}
}
[Subscribe(With = nameof(SubItems))]
public Item OnItem([Message] Item item) {
return item;
}
produces this schema
type Subscriptions {
onItem: Item!;
}
As we can see, signature for node onItem is reived from method OnItem. As this method is parameterless so is the node. However subscribe function SubItems requires a parameter. If we run the code (breakpoint on await foreach line), as soon as someone open a channel we see that we did not break there. In this case HC silently fails.
If we adjust second method to look like
[Subscribe(With = nameof(SubItems))]
public Item OnItem(long id, [Message] Item item) {
return item;
}
schema becomes
type Subscriptions {
onItem(id: Long!): Item!;
}
and as soon as channel opens we break on breakpoint from above. If we passed for example 8, we can observer that id in boh methods holds an 8. In this case HC works
If we change name of the parameter id of one to notId, we no longer break there and again HC silently fails. So do mismatches in parameter types, but only and only if the parameters types (and values) are not compatible (for example int and long work together but only if the value provided is not outside int range so value 1844674407370955161 fails silently).
Steps to reproduce
- Above
Relevant log output
No response
Additional Context?
No response
Product
Hot Chocolate
Version
12.9.0
Other question is whether requirement to duplicate parameters on each method is not bug as well. Resolver method does not do anything with the id, so it should not be necessary to pass it there. I think that node's signature should be created by merging both methods' signatures and distributing values accordingly.
Its actually not a bug ... since the resolver is what defines the field. the factory can just also access the arguments. But we are changing this with 13.
Ok but I do think that it should not fail silently eve in 12. Either it should throw at startup or during invocation.
It's not so simple to achieve this since if we could do that we could infer the and merge. So, for now it is as it is and works in its limitations. For V13 this will change this. For V12 there will be no change... the gap between 12 and 13 is huge on subscriptions.
But if you want you can add a note to the docs outlining this limitation.