Fix exception in NFT subscription

[IvanIvanoff]

Changes

Ticket

Checklist:

• I have performed a self-review of my own code
• I have made corresponding changes to the documentation
• I have tried to find clearer solution before commenting hard-to-understand parts of code
• I have added tests that prove my fix is effective or that my feature works

[Copilot]

Pull request overview

This PR adds comprehensive error handling to the NFT subscription module to prevent exceptions when smart contract calls fail. The changes gracefully handle errors from Ethereum contract calls and log warnings instead of crashing.

• Added error handling with pattern matching for execute() and execute_batch() function calls
• Introduced logging for failed contract calls to aid debugging
• Added nil filtering to handle failed token ID fetches

Comments suppressed due to low confidence (1)

lib/sanbase/smart_contracts/sanbase_nft.ex:55

• The new error handling logic introduced in this function lacks test coverage. The existing tests in test/sanbase_web/graphql/sanbase_nft_api_test.exs mock the nft_subscriptions_data function at a high level and don't verify the behavior of the new error handling paths.

Consider adding unit tests that verify:

1. The function correctly handles {:error, reason} responses from execute("tokenOfOwnerByIndex", ...)
2. The function correctly handles {:error, reason} responses from execute("isValid", ...)
3. Nil values are properly filtered from the token_ids list
4. The function continues processing remaining tokens when individual calls fail

  def nft_subscriptions_data(address) do
    tokens_count = balance_of(address)

    if tokens_count > 0 do
      token_ids =
        0..(tokens_count - 1)
        |> Enum.map(fn idx ->
          address = format_address(address)

          case execute("tokenOfOwnerByIndex", [address, idx]) do
            {:error, reason} ->
              Logger.warning("Failed to fetch token ID at index #{idx}: #{inspect(reason)}")
              nil

            result when is_list(result) ->
              List.first(result)
          end
        end)
        |> Enum.filter(&(&1 != nil))

      valid_token_ids =
        Enum.filter(token_ids, fn token_id ->
          case execute("isValid", [token_id]) do
            {:error, reason} ->
              Logger.warning("Failed to check validity for token #{token_id}: #{inspect(reason)}")
              false

            result when is_list(result) ->
              hd(result)
          end
        end)

      non_valid_token_ids = token_ids -- valid_token_ids
      %{valid: valid_token_ids, non_valid: non_valid_token_ids}
    else
      %{valid: [], non_valid: []}
    end
  end

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The pattern matching order in this case statement is problematic. The pattern [balance] will successfully match a list with one element, but {:error, reason} will only match error tuples. However, these patterns are mutually exclusive, so the current order could work.

The issue is that this ordering is inconsistent with the error handling pattern used in the other functions in this file (like tokens_of_owner at lines 27-34 and 40-47), where the error case is checked first. For consistency and clarity, the {:error, reason} pattern should be checked before the success case [balance].

Suggested change

	[balance] ->
	balance
	{:error, reason} ->
	Logger.warning("Failed to fetch NFT balance for address: #{inspect(reason)}")
	0
	{:error, reason} ->
	Logger.warning("Failed to fetch NFT balance for address: #{inspect(reason)}")
	0
	[balance] ->
	balance

[Copilot]

The error handling here is incomplete. The execute_batch function (which calls call_contract_batch) can return nil when the batch_request fails at the outer level (when the with statement in call_contract_batch doesn't match). This case is not handled in the pattern matching.

Add a nil pattern to handle this case, similar to how you handle other error conditions. For example:

nil ->
  Logger.warning("Failed to fetch NFT balances: batch request failed")
  List.duplicate(0, length(addresses))

[Copilot]

The new error handling logic in this function lacks test coverage. Consider adding unit tests that verify the function correctly handles {:error, reason} responses from execute_batch("balanceOf", ...) and returns a list of zeros with the correct length.

[Copilot]

The new error handling logic in this function lacks test coverage. Consider adding unit tests that verify the function correctly handles {:error, reason} responses from execute("balanceOf", ...) and returns 0 as expected.