paritytech / capi

[WIP] A framework for crafting interactions with Substrate chains
https://docs.capi.dev
Apache License 2.0
104 stars 9 forks source link

Querying a double map with a partial key results in an exception #1169

Closed rflechtner closed 1 year ago

rflechtner commented 1 year ago

Bug Report

Querying a double map with a partial key throws if there are no results on v0.1.0-gamma.1.

Current Behavior

Throws exception due to changeset being undefined.

[...]/node_modules/capi/esm/fluent/StorageRune.js:39
            .map(([changeset, $key, $value]) => changeset.changes.map(([k, v]) => [

TypeError: Cannot read properties of undefined (reading 'changes')
    at RunMap.fn (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/fluent/StorageRune.js:39:59)
    at RunMap._evaluate (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/rune/ValueRune.js:117:44)
    at processTicksAndRejections (node:internal/process/task_queues:95:5)
    from execution of the ValueRune instantiated
        at new Rune (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/rune/Rune.js:44:23)
        at new ValueRune (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/rune/ValueRune.js:4:8)
        at Function.new (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/rune/ValueRune.js:6:16)
        at ValueRune.map (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/rune/ValueRune.js:9:29)
        at StorageRune.entries (file:///Users/raphael/Code/Kilt/capi%20example/node_modules/capi/esm/fluent/StorageRune.js:39:14)
        at file:///Users/raphael/Code/Kilt/capi%20example/reproduce.js:10:50
        at processTicksAndRejections (node:internal/process/task_queues:95:5)

Expected Behavior

Returns empty array.

Steps To Reproduce

nets.js

import { net } from "capi/nets";

export const spiritnet = net.ws({ url: "wss://spiritnet.kilt.io" });

reproduce.js

import { spiritnet } from "@capi/spiritnet";
import { ss58 } from "capi";

// works
console.log(await spiritnet.Did.ServiceEndpoints.entries({
    partialKey: [ss58.decode('4sJm5Zsvdi32hU88xbL3v6VQ877P4HLaWVYUXgcSyQR8URTu')[1]],
}).run())

// breaks
console.log(await spiritnet.Did.ServiceEndpoints.entries({
    partialKey: [ss58.decode('4saFtHZuY9UH1U7zasKLSkKsPogJjPqM1vT2k3ZJmrYmmr3g')[1]],
}).run())

Environment

harrysolovay commented 1 year ago

In the second case, seems there are no entries associated with the partial key, hence no changes within the queried changeset. #1170 should fix / will return an empty list.

Please confirm that the second should in fact be an empty list.

rflechtner commented 1 year ago

Confirmed, just as with {limit: 0} or any other query that produces a result of size 0, I'd expect an empty list - this is easier to handle than, for example, returning undefined instead. Not sure about what the right runic type would be for that - is it a ArrayRune or a ValueRune of an array? Rn this seems to be typed as a plain Rune type though, which is a bit cumbersome as we don't get access to the arrayMap or map methods that allow transforming the results.

harrysolovay commented 1 year ago

For many use cases, you can just run and operate on the result.

const entries = await spiritnet.Did.ServiceEndpoints
  .entries({
    partialKey: [ss58.decode('4saFtHZuY9UH1U7zasKLSkKsPogJjPqM1vT2k3ZJmrYmmr3g')[1]],
  })
  .run()

const mapped = entries.map(cb)

For larger runes (such as derived reads and workflows), staying within the rune system preserves intermediate error types and ensures certain values are memoized.

For these cases, you can use the into method––which is accessible on all runes––to get a rune of the desired subclass.

const mapped = await spiritnet.Did.ServiceEndpoints
  .entries({
    partialKey: [ss58.decode('4saFtHZuY9UH1U7zasKLSkKsPogJjPqM1vT2k3ZJmrYmmr3g')[1]],
  })
  .into(ArrayRune)
  .mapArray(cb)
  .run()
rflechtner commented 1 year ago

Yes, I figured that out! That works of course. I only brought it up because I saw that entries is the only one among related methods such as keys and entriesRaw that is typed as a plain Rune type, so I figured that may have been an oversight.

image

harrysolovay commented 1 year ago

You're right, returning as a ValueRune would be preferable, as this means util methods such as map and select are immediately available. Just updated the PR to do the into(ValueRune) under-the-hood. Nice catch!

harrysolovay commented 1 year ago

Just triggered a release of v0.1.1-beta.1. It's been published to NPM and Denoland/x.

rflechtner commented 1 year ago

Just triggered a release of v0.1.1-beta.1. It's been published to NPM and Denoland/x.

fixed the issue for me :)