graphql-core-legacy icon indicating copy to clipboard operation
graphql-core-legacy copied to clipboard

Cannot subscribe to more than one field

Open jadamsBG opened this issue 7 years ago • 1 comments

Attempting to subscribe to more than one field will only return the first one in the list. There is some obvious offending code in executor.py: `def subscribe_fields(exe_context, parent_type, source_value, fields): exe_context = SubscriberExecutionContext(exe_context)

def on_error(error):
    exe_context.report_error(error)

def map_result(data):
    if exe_context.errors:
        result = ExecutionResult(data=data, errors=exe_context.errors)
    else:
        result = ExecutionResult(data=data)
    exe_context.reset()
    return result

observables = []

# assert len(fields) == 1, "Can only subscribe one element at a time."

for response_name, field_asts in fields.items():
    result = subscribe_field(exe_context, parent_type, source_value, field_asts, [response_name])
    if result is Undefined:
        continue

    def catch_error(error):
        exe_context.errors.append(error)
        return Observable.just(None)

    # Map observable results
    observable = result.catch_exception(catch_error).map(
        lambda data: map_result({response_name: data}))
    return observable
    observables.append(observable)

return Observable.merge(observables)

`

The line return observable causes a bunch of unreachable code and exits the loop early. However, commenting out that line does not appear to fix the problem - if it is removed, only the last item in the subscription list will return.

jadamsBG avatar Jun 28 '18 19:06 jadamsBG

To solve this, one would need to dig deep into the subscription code and add tests for subscribing to multiple fields (or improve the existing test_accepts_multiple_subscription_fields_defined_in_schema).

Not sure if it's worthwile since this is legacy code - current development happens in GraphQL core 3. But leaving this open for anybody who is willing to tackle this.

Cito avatar Jan 24 '20 22:01 Cito