Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

retrieving nonexistent agreement record breaks resolver #399

Open
LeosPrograms opened this issue Jan 26, 2024 · 2 comments
Open

retrieving nonexistent agreement record breaks resolver #399

LeosPrograms opened this issue Jan 26, 2024 · 2 comments
Assignees
Labels
bug Something isn't working

Comments

@LeosPrograms
Copy link
Contributor

using
clauseOf {
id
}
for records that don't have caluseOf causes the error

ERROR wasm_trace: hc_zome_rea_agreement::__get_agreement_extern:zomes/rea_agreement/zome/src/lib.rs:31 output_type = "core::result::Result<hc_zome_rea_agreement_rpc::ResponseData, holochain_wasmer_common::result::WasmError>"; bytes = [129, 167, 97, 100, 100, 114, 101, 115, 115, 192]; Deserialize("invalid type: unit value, expected tuple struct AgreementAddress")

this appears to be due to an attempt to return a record for a blank agreement address

@LeosPrograms LeosPrograms added the bug Something isn't working label Jan 26, 2024
@pospi
Copy link
Member

pospi commented Jan 28, 2024

Is there a failing test for this anywhere at present, or do we need to add coverage? No failure in test_agreement_links.ts- good catch! I think this might be an issue that affects many record types.

Looks like the underlying symptom is a deserialization error being thrown for the input- i.e. the call to get_agreement is being sent a null value rather than an AgreementAddress. So yea what you said (; I am guessing in an app you are using Commitment.clauseOf against a list of Commitments rather than a single one that's known to have an associated Agreement?

See if altering this line for commitment to return record.clauseOf ? ... : null fixes the error. That should give the expected behaviour to avoid calling the zome API if there is no associated value to send.

If that solves the bug, can you please take a look through all resolvers/ and add the same check for any resolver that calls an API with a Maybe<?> value? Your IDE should make it easy to tell which ones are optional, eg. VSCode shows a tooltip with Maybe<Agreement> | undefined when I hover record.clauseOf for Commitment.

It would probably also be best practise to add some failing tests for these but shrug this is actually something that should be caught at the typechecker level (resolvers/commitment pulls the queries/agreement and the latter does not have a type signature for args in agreement: async (root, args)). I am happy to pop a couple of tests in for Agreement and leave it at that?

@pospi
Copy link
Member

pospi commented Jan 28, 2024

I just popped a test in, but haven't been able to successfully run it 'cos my conductor is timing out and I don't have time to debug that atm. Hopefully it fails when you run it and making the above changes fixes it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants