Incomplete error response on mutation with multiple instances of the same mutation

[lucianoengel]

Environment

• Elixir version (elixir -v): 1.10.4
• Absinthe version (mix deps | grep absinthe): 1.6.4
• Client Framework and version (Relay, Apollo, etc): Apollo

Expected behavior

Currently our client makes a single request like this:

mutation placeLimitOrders($affiliateDeveloperCode: AffiliateDeveloperCode, $pA:PlaceLimitOrderParams!, $sA:Signature!,$pB:PlaceLimitOrderParams!, $sB:Signature!) {
  A: placeLimitOrder(affiliateDeveloperCode: $affiliateDeveloperCode, payload: $pA, signature: $sA) {
  id
  status
  ordersTillSignState
 }
  B: placeLimitOrder(affiliateDeveloperCode: $affiliateDeveloperCode, payload: $pB, signature: $sB) {
  id
  status
  ordersTillSignState
 }
}

we expect that if one or more of these mutations fails we get individual errors for every mutation, and that an error in one of them doesn't block the others from being executed.

Actual Behavior

If there's an API error, like a missing field etc, it does work as expected and the result we get is this one

[
  {
    locations: [ [Object] ],
    message: 'Argument "payload" has invalid value $pA.\n' +
      'In field "cancellationPolicy": Expected type "OrderCancellationPolicy!", found null.'
  },
  {
    locations: [ [Object] ],
    message: 'Argument "payload" has invalid value $pB.\n' +
      'In field "cancellationPolicy": Expected type "OrderCancellationPolicy!", found null.'
  }
]

But, if there's an error coming from a resolver function it seems that the execution is halted and the error returned immediately. This is what we get:

[
  {
    locations: [ [Object] ],
    message: "validation error caused by 'account_funds' - requirement:  - reason: Insufficient Funds - got: ~C[2.00000000 btc]",
    path: [ 'A' ]
  }
]

And it seems the operation B doesn't ever run....

[benwilson512]

Are you able to reproduce this error with just Absinthe.run? master has a commit that demonstrates that this should work, so I need an example I can run to proceed.

[lucianoengel]

will try to get you something

[lucianoengel]

so I was able to reproduce this in this test

  @multiple_place_limit_order """
  mutation($payloadA: PlaceLimitOrderParams!, $signature: Signature!, $payloadB: PlaceLimitOrderParams!, $affiliate: AffiliateDeveloperCode) {
    A: placeLimitOrder(payload: $payloadA, signature: $signature, affiliateDeveloperCode: $affiliate) {
      id
    }
    B: placeLimitOrder(payload: $payloadB, signature: $signature, affiliateDeveloperCode: $affiliate) {
      id
    }
  }
  """
  @tag order_book: {:gas, :neo}
  @tag authentication: :access,
       include_wallets: true,
       account: %{identity: %{country_code: :GB, kyc_status: :approved_sanction_screening}}
  test "placeLimitOrder - multi call errors", ctx do
    neo_account = TestUtil.get_account(ctx, :neo)
    timestamp = now_ms()
    nonce = rem(timestamp, 1 <<< 32)

    payload = %{
      timestamp: timestamp,
      market_name: "gas_neo",
      buy_or_sell: "BUY",
      amount: %{amount: "10.0", currency: "gas"},
      limit_price: %{amount: "0.1", currency_a: "neo", currency_b: "gas"},
      allow_taker: true,
      cancellation_policy: "GOOD_TIL_CANCELLED",
      nonce_from: 0,
      nonce_to: 0,
      nonce_order: nonce
    }

    payload = SignatureHelpers.add_signatures_graphql(payload, %{neo_account: neo_account})

    payload_a = %{
      "allowTaker" => payload.allow_taker,
      "amount" => %{"amount" => payload.amount.amount, "currency" => payload.amount.currency},
      "blockchainSignatures" => payload.blockchain_signatures,
      "buyOrSell" => payload.buy_or_sell,
      "cancellationPolicy" => payload.cancellation_policy,
      "limitPrice" => %{
        "amount" => payload.amount.amount,
        "currencyA" => payload.limit_price.currency_a,
        "currencyB" => payload.limit_price.currency_b
      },
      "marketName" => payload.market_name,
      "nonceFrom" => payload.nonce_from,
      "nonceOrder" => payload.nonce_order,
      "nonceTo" => payload.nonce_to,
      "timestamp" => payload.timestamp
    }

    payload_b = Map.put(payload_a, "nonceOrder", nonce + 1)

    signature = %{
      "publicKey" => "72727768",
      "signedDigest" => "6f61646a6f64616a646f61736a646f696a64"
    }

    {:ok, %{errors: errors}} =
      Absinthe.run(@multiple_place_limit_order, CasApiWeb.Schema.Schema,
        variables: %{
          "payloadA" => payload_a,
          "signature" => signature,
          "payloadB" => payload_b,
          "affiliate" => "test"
        },
       context: ctx
      )

    assert length(errors) == 2
  end

which fails by having only a single error, for path A like the response we get in our API's What I did notice is that it seems I was wrong and the resolvers are getting called twice, and they both are returning errors.

I know you still can't run this, but would appreciate any ideas you might have as I keep investigating this

[benwilson512]

OK, I'll see what I can do to reproduce. Oh one last question, can you show the field definition for that mutation?

[lucianoengel]

sure! Thanks, appreciate. Let me know If I can get you anything else

    @desc "Place a new limit order"
    field :place_limit_order, non_null(:order_placed) do
      arg(:payload, non_null(:place_limit_order_params))
      arg(:signature, non_null(:signature))
      arg(:affiliate_developer_code, :affiliate_developer_code)

      middleware(RateLimiter,
        name: "place_limit_order",
        attempts: @attempts_per_second,
        duration: @duration_in_ms
      )

      middleware(Authorize, [:access, :api_trade])
      middleware(BlockIncompleteAccount)
      middleware(BlockDuplicateAccount)
      middleware(MpcCompleteBlockchainSignatures, req_type: "place_limit_order")
      middleware(ReplayProtection.Check)

      resolve(&AccountOrder.place_limit_order/3)

      middleware(ReplayProtection.Store)
    end

[benwilson512]

This is the main issue. non_null(:order_placed). Fields that return errors should not be marked non_null. When you return an error on the field, its value is always null. When a null constraint error happens, the null is propagated upwards until the first nullable key (or the data key) which will end up hiding sibling fields. Absinthe can't report errors on those sibling fields because they're hidden. Now, I should definitely make sure that Absinthe is doing the right thing here in this relatively complex combination of features.

However you can solve your specific problem by removing the non_null on that mutation.

[lucianoengel]

Alright, will do that! Thanks @benwilson512 really appreciate the time!