fix subscriptuon bug - #442
Conversation
|
@barnabasJ could you potentially take a look at this? Looks reasonable to me. |
barnabasJ
left a comment
barnabasJ
left a comment
There was a problem hiding this comment.
@aheiner2001 the new test is already green with the resover / batcher implementation from main. could you describe what problem you were trying to solve exactly?
|
@barnabasJ .I was fixing issue #354: when a subscription DSL filter excludes an event, the subscriber should get nothing, not data: null with a not_found error. On main, the new test already passes because the batcher suppresses not_found before publish, even though the resolver still treats filter exclusion as NotFound. My change makes exclusion mean “drop” in the resolver ({:ok, nil}) and updates the batcher so null-only payloads aren’t sent either — those two parts need to work together. |
Ah, I missed the referenced issue, I'll have another look over the weekend |
|
@aheiner2001 I had another look at the issue as well, but couldn't really reproduce it. Were you able to make it fail locally the way the issue is described? I don't necessarily see a problem with the changes, but it would be great to have a test that actually triggers the issue mentioned. |
|
@barnabasJ From what I can see, this is the initial bug reported. |
|
@aheiner2001 looked at the reproduction repo, and I think the actual issue is in the igniter installer Line 270 in a0d8b9d AshGraphql.Subscription.Endpoint, otherwise our filter logic will not run.
|
… run The installer added "use Absinthe.Phoenix.Endpoint", which doesn't delegate run_docset/3 to AshGraphql.Subscription.Runner, so the subscription batcher/filter logic never ran in generated apps. This caused filtered-out subscription events to be delivered as null + not_found (issue ash-project#354).
|
@barnabasJ You're right — I've reverted my initial changes and implemented the ones you suggested. |
3 participants
Contributor checklist
Leave anything that you believe does not apply unchecked.