Simplify 'last call' logic for queryAndUpdate (dfinity#5563)

# Motivation

With `queryAndUpdate` we make 2 calls but sometimes we only want to do
something for 1 of the calls.
For example, if there is an error in the query call, we just ignore it
and wait for the update call.

The problem is that `queryAndUpdate` doesn't always do 2 calls and there
is some logic to determine whether to do 2 calls or not. If the user is
not logged in, we only do a query call and it's also possible to pass in
a strategy, which might also depend on the value of
`FORCE_CALL_STRATEGY`.
Altogether this makes knowing which strategy is currently being used,
and thus whether we are doing 1 or 2 calls, and thus whether the current
call is the last call or if another call is still following, quite
fragile.

In this PR, we make `queryAndUpdate` pass the actually used strategy to
the `onLoad` and `onError` callback and also add a `isLastCall` function
to determine if the current call is either the update call of
query+update or the only call of a single query or update call.

# Changes

1. Make `queryAndUpdate` pass `strategy` to its callbacks.
2. Add `isLastCall` utility function.
3. Use the utility function in all the places where it makes sense.

# Tests

1. Add unit tests for `isLastCall`.
2. Update unit tests for `queryAndUpdate`.
3. Add a lot of missing test coverage because it seems that all these
"is this the last call" code paths were untested.

# Todos

- [ ] Add entry to changelog (if necessary).
not necessary

frontend/src/lib/services/canisters.services.ts

@@ -22,7 +22,7 @@ import type { Account } from "$lib/types/account";
 import { LedgerErrorMessage } from "$lib/types/ledger.errors";
 import { assertEnoughAccountFunds } from "$lib/utils/accounts.utils";
 import { isController } from "$lib/utils/canisters.utils";
-import { notForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import {
   mapCanisterErrorToToastMessage,
   toToastError,
@@ -48,10 +48,10 @@ export const listCanisters = async ({
     strategy: FORCE_CALL_STRATEGY,
     onLoad: ({ response: canisters, certified }) =>
       canistersStore.setCanisters({ canisters, certified }),
-    onError: ({ error: err, certified }) => {
+    onError: ({ error: err, certified, strategy }) => {
       console.error(err);
 
-      if (!certified && notForceCallStrategy()) {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 

frontend/src/lib/services/ckbtc-info.services.ts

@@ -5,7 +5,7 @@ import { ckBTCInfoStore } from "$lib/stores/ckbtc-info.store";
 import { toastsError } from "$lib/stores/toasts.store";
 import type { CkBTCAdditionalCanisters } from "$lib/types/ckbtc-canisters";
 import type { UniverseCanisterId } from "$lib/types/universe";
-import { notForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import { isUniverseCkBTC } from "$lib/utils/universe.utils";
 import type { MinterInfo } from "@dfinity/ckbtc";
 import { isNullish } from "@dfinity/utils";
@@ -50,8 +50,8 @@ export const loadCkBTCInfo = async ({
         canisterId: universeId,
         info,
       }),
-    onError: ({ error: err, certified }) => {
-      if (!certified && notForceCallStrategy()) {
+    onError: ({ error: err, certified, strategy }) => {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 

frontend/src/lib/services/icrc-accounts.services.ts

@@ -20,6 +20,7 @@ import { toastsError } from "$lib/stores/toasts.store";
 import { tokensStore } from "$lib/stores/tokens.store";
 import type { Account } from "$lib/types/account";
 import type { IcrcTokenMetadata } from "$lib/types/icrc";
+import { isLastCall } from "$lib/utils/env.utils";
 import { toToastError } from "$lib/utils/error.utils";
 import { isImportedToken } from "$lib/utils/imported-tokens.utils";
 import { ledgerErrorToToastError } from "$lib/utils/sns-ledger.utils";
@@ -79,10 +80,8 @@ export const loadIcrcToken = ({
     return;
   }
 
-  const strategy = certified ? FORCE_CALL_STRATEGY : "query";
-
   return queryAndUpdate<IcrcTokenMetadata, unknown>({
-    strategy,
+    strategy: certified ? FORCE_CALL_STRATEGY : "query",
     identityType: "current",
     request: ({ certified, identity }) =>
       queryIcrcToken({
@@ -92,9 +91,9 @@ export const loadIcrcToken = ({
       }),
     onLoad: async ({ response: token, certified }) =>
       tokensStore.setToken({ certified, canisterId: ledgerCanisterId, token }),
-    onError: ({ error: err, certified }) => {
+    onError: ({ error: err, certified, strategy }) => {
       // Explicitly handle only UPDATE errors
-      if (!certified && strategy !== "query") {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 

frontend/src/lib/services/imported-tokens.services.ts

@@ -17,7 +17,7 @@ import {
 } from "$lib/stores/imported-tokens.store";
 import { toastsError, toastsSuccess } from "$lib/stores/toasts.store";
 import type { ImportedTokenData } from "$lib/types/imported-tokens";
-import { notForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import {
   fromImportedTokenData,
   toImportedTokenData,
@@ -45,7 +45,7 @@ export const loadImportedTokens = async ({
       });
       failedImportedTokenLedgerIdsStore.reset();
     },
-    onError: ({ error: err, certified }) => {
+    onError: ({ error: err, certified, strategy }) => {
       console.error(err);
 
       if (ignoreAccountNotFoundError && err instanceof AccountNotFoundError) {
@@ -59,7 +59,7 @@ export const loadImportedTokens = async ({
         return;
       }
 
-      if (!certified && notForceCallStrategy()) {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 

frontend/src/lib/services/neurons.services.ts

@@ -30,7 +30,7 @@ import {
   assertEnoughAccountFunds,
   isAccountHardwareWallet,
 } from "$lib/utils/accounts.utils";
-import { notForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import {
   errorToString,
   mapNeuronErrorToToastMessage,
@@ -266,8 +266,8 @@ export const listNeurons = async ({
 
       callback?.(certified);
     },
-    onError: ({ error, certified }) => {
-      if (!certified && notForceCallStrategy()) {
+    onError: ({ error, certified, strategy }) => {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 
@@ -908,10 +908,10 @@ export const loadNeuron = ({
       }
       setNeuron({ neuron, certified });
     },
-    onError: ({ error, certified }) => {
+    onError: ({ error, certified, strategy }) => {
       console.error(error);
 
-      if (!certified && notForceCallStrategy()) {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
       catchError(error);

frontend/src/lib/services/public/proposals.services.ts

@@ -18,14 +18,14 @@ import {
 } from "$lib/stores/proposals.store";
 import { toastsError, toastsShow } from "$lib/stores/toasts.store";
 import { hashCode } from "$lib/utils/dev.utils";
-import { isForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import { errorToString, isPayloadSizeError } from "$lib/utils/error.utils";
 import {
   excludeProposals,
   proposalsHaveSameIds,
 } from "$lib/utils/proposals.utils";
-import type { Identity } from "@dfinity/agent";
 import type { ProposalId, ProposalInfo } from "@dfinity/nns";
+import { nonNullish } from "@dfinity/utils";
 import { get } from "svelte/store";
 import { getCurrentIdentity } from "../auth.services";
 import {
@@ -38,20 +38,16 @@ import {
 const handleFindProposalsError = ({
   error: err,
   certified,
-  identity,
+  strategy,
   mutableProposalsStore,
 }: {
   error: unknown;
   certified: boolean;
-  identity: Identity;
+  strategy: QueryAndUpdateStrategy;
   mutableProposalsStore: SingleMutationProposalsStore;
 }) => {
   console.error(err);
-  if (
-    certified ||
-    identity.getPrincipal().isAnonymous() ||
-    isForceCallStrategy()
-  ) {
+  if (isLastCall({ strategy, certified })) {
     mutableProposalsStore.setProposals({ proposals: [], certified });
 
     const resultsTooLarge = isPayloadSizeError(err);
@@ -88,7 +84,7 @@ export const listProposals = async ({
     onError: (onErrorParams: {
       error: unknown;
       certified: boolean;
-      identity: Identity;
+      strategy: QueryAndUpdateStrategy;
     }) => {
       handleFindProposalsError({
         ...onErrorParams,
@@ -136,7 +132,7 @@ export const listNextProposals = async ({
     onError: (onErrorParams: {
       error: unknown;
       certified: boolean;
-      identity: Identity;
+      strategy: QueryAndUpdateStrategy;
     }) => {
       handleFindProposalsError({
         ...onErrorParams,
@@ -163,7 +159,7 @@ const findProposals = async ({
   const validateResponses = ({
     trustedProposals,
     untrustedProposals,
-  }:{
+  }: {
     trustedProposals: ProposalInfo[];
     untrustedProposals: ProposalInfo[];
   }) => {
@@ -203,10 +199,10 @@ const findProposals = async ({
         includeStatus: filters.status,
         certified,
       }),
-    onLoad: ({ response: proposals, certified }) => {
+    onLoad: ({ response: proposals, certified, strategy }) => {
       if (!certified) {
         uncertifiedProposals = proposals;
-        onLoad({ response: proposals, certified });
+        onLoad({ response: proposals, certified, strategy });
         return;
       }
 
@@ -217,7 +213,7 @@ const findProposals = async ({
         });
       }
 
-      onLoad({ response: proposals, certified });
+      onLoad({ response: proposals, certified, strategy });
     },
     onError,
     logMessage: `Syncing proposals ${
@@ -252,12 +248,14 @@ export const loadProposal = async ({
   ) => {
     console.error(erroneusResponse);
 
-    if (silentErrorMessages !== true && (
-      erroneusResponse.certified || (
-        strategy === "query" ||
-        identity.getPrincipal().isAnonymous()
-      )
-    )) {
+    if (
+      silentErrorMessages !== true &&
+      nonNullish(erroneusResponse) &&
+      isLastCall({
+        strategy: erroneusResponse.strategy,
+        certified: erroneusResponse.certified,
+      })
+    ) {
       const details = errorToString(erroneusResponse?.error);
       toastsShow({
         labelKey: "error.proposal_not_found",
@@ -274,9 +272,9 @@ export const loadProposal = async ({
   try {
     return await getProposal({
       proposalId,
-      onLoad: ({ response: proposal, certified }) => {
+      onLoad: ({ response: proposal, certified, strategy }) => {
         if (!proposal) {
-          catchError({ certified, error: undefined, identity });
+          catchError({ certified, strategy, error: undefined, identity });
           return;
         }
 
@@ -288,7 +286,15 @@ export const loadProposal = async ({
       strategy,
     });
   } catch (error: unknown) {
-    catchError({ certified: true, error, identity });
+    catchError({
+      // `certified` and `strategy` are not meaningful since we are outside the
+      // `queryAndUpdate` context. We just pass values that act as if this is
+      // the last call.
+      certified: true,
+      strategy: "query_and_update",
+      error,
+      identity,
+    });
   }
 };
 

frontend/src/lib/services/public/sns.services.ts

@@ -6,7 +6,7 @@ import { queryAndUpdate } from "$lib/services/utils.services";
 import { snsAggregatorIncludingAbortedProjectsStore } from "$lib/stores/sns-aggregator.store";
 import { snsProposalsStore } from "$lib/stores/sns.store";
 import { toastsError } from "$lib/stores/toasts.store";
-import { isForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import { toToastError } from "$lib/utils/error.utils";
 import { ProposalStatus, Topic, type ProposalInfo } from "@dfinity/nns";
 import { Principal } from "@dfinity/principal";
@@ -79,13 +79,11 @@ export const loadProposalsSnsCF = async (): Promise<void> => {
         proposals,
         certified,
       }),
-    onError: ({ error: err, certified, identity }) => {
+    onError: ({ error: err, certified, strategy }) => {
       console.error(err);
 
       if (
-        certified ||
-        identity.getPrincipal().isAnonymous() ||
-        isForceCallStrategy()
+        isLastCall({ strategy, certified })
       ) {
         snsProposalsStore.reset();
 

frontend/src/lib/services/sns-neurons.services.ts

@@ -31,7 +31,7 @@ import {
 } from "$lib/stores/sns-neurons.store";
 import { toastsError, toastsSuccess } from "$lib/stores/toasts.store";
 import type { Account } from "$lib/types/account";
-import { notForceCallStrategy } from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import { toToastError } from "$lib/utils/error.utils";
 import { ledgerErrorToToastError } from "$lib/utils/sns-ledger.utils";
 import {
@@ -89,10 +89,10 @@ export const syncSnsNeurons = async (
         certified,
       });
     },
-    onError: ({ error: err, certified }) => {
+    onError: ({ error: err, certified, strategy }) => {
       console.error(err);
 
-      if (!certified && notForceCallStrategy()) {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 

frontend/src/lib/services/sns.services.ts

@@ -14,10 +14,7 @@ import {
 } from "$lib/stores/sns.store";
 import { toastsError } from "$lib/stores/toasts.store";
 import type { SnsSwapCommitment } from "$lib/types/sns";
-import {
-  isForceCallStrategy,
-  notForceCallStrategy,
-} from "$lib/utils/env.utils";
+import { isLastCall } from "$lib/utils/env.utils";
 import { toToastError } from "$lib/utils/error.utils";
 import { getSwapCanisterAccount } from "$lib/utils/sns.utils";
 import type { AccountIdentifier } from "@dfinity/ledger-icp";
@@ -73,10 +70,10 @@ export const loadSnsSwapCommitments = async (): Promise<void> => {
         });
       }
     },
-    onError: ({ error: err, certified }) => {
+    onError: ({ error: err, certified, strategy }) => {
       console.error(err);
 
-      if (!certified && notForceCallStrategy()) {
+      if (!isLastCall({ strategy, certified })) {
         return;
       }
 
@@ -123,14 +120,10 @@ export const loadSnsSwapCommitment = async ({
       }),
     onLoad: ({ response: swapCommitment, certified }) =>
       snsSwapCommitmentsStore.setSwapCommitment({ swapCommitment, certified }),
-    onError: ({ error: err, certified, identity }) => {
+    onError: ({ error: err, certified, strategy }) => {
       console.error(err);
 
-      if (
-        certified ||
-        identity.getPrincipal().isAnonymous() ||
-        isForceCallStrategy()
-      ) {
+      if (isLastCall({ strategy, certified })) {
         toastsError(
           toToastError({
             err,

frontend/src/lib/services/utils.services.ts

@@ -8,11 +8,13 @@ import {
 
 export type QueryAndUpdateOnResponse<R> = (options: {
   certified: boolean;
+  strategy: QueryAndUpdateStrategy;
   response: R;
 }) => void;
 
 export type QueryAndUpdateOnError<E> = (options: {
   certified: boolean;
+  strategy: QueryAndUpdateStrategy;
   error: E;
   // The identity used for the request
   identity: Identity;
@@ -83,12 +85,12 @@ export const queryAndUpdate = async <R, E>({
     request({ certified, identity })
       .then((response) => {
         if (certifiedDone) return;
-        onLoad({ certified, response });
+        onLoad({ certified, strategy: currentStrategy, response });
         log({ postfix: ` ${certified ? "update" : "query"} complete.` });
       })
       .catch((error: E) => {
         if (certifiedDone) return;
-        onError?.({ certified, error, identity });
+        onError?.({ certified, strategy: currentStrategy, error, identity });
       })
       .finally(() => (certifiedDone = certifiedDone || certified));