fix(trading): error guards not working when poll interval is supplied (#5661)

Co-authored-by: Madalina Raicu <madalina@raygroup.uk>
Co-authored-by: Dariusz Majcherczyk <dariusz.majcherczyk@gmail.com>

apps/trading/e2e/tests/get_started/test_get_started.py

@@ -10,15 +10,18 @@ import logging
 
 logger = logging.getLogger()
 
+
 @pytest.fixture(scope="class")
 def vega():
     with init_vega() as vega:
         yield vega
 
+
 @pytest.fixture(scope="class")
 def simple_market(vega: VegaServiceNull):
     return setup_simple_market(vega)
 
+
 class TestGetStarted:
     def test_get_started_interactive(self, vega: VegaServiceNull, page: Page):
         page.goto("/")
@@ -30,7 +33,8 @@ class TestGetStarted:
         expect(page.locator(".list-none")).to_contain_text(
             "1.Connect2.Deposit funds3.Open a position"
         )
-        DEFAULT_WALLET_NAME = "MarketSim"  # This is the default wallet name within VegaServiceNull and CANNOT be changed
+        # This is the default wallet name within VegaServiceNull and CANNOT be changed
+        DEFAULT_WALLET_NAME = "MarketSim"
 
         # Calling get_keypairs will internally call _load_tokens for the given wallet
         keypairs = vega.wallet.get_keypairs(DEFAULT_WALLET_NAME)
@@ -137,7 +141,8 @@ class TestGetStarted:
     def test_get_started_seen_already(self, simple_market, page: Page):
         page.goto(f"/#/markets/{simple_market}")
         get_started_locator = page.get_by_test_id("connect-vega-wallet")
-        page.wait_for_selector('[data-testid="connect-vega-wallet"]', state="attached")
+        page.wait_for_selector(
+            '[data-testid="connect-vega-wallet"]', state="attached")
         expect(get_started_locator).to_be_enabled
         expect(get_started_locator).to_be_visible
         # 0007-FUGS-015

apps/trading/e2e/tests/market/test_market_info.py

@@ -36,16 +36,19 @@ def validate_info_section(page: Page, fields: [[str, str]]):
     for rowNumber, field in enumerate(fields):
         name, value = field
         expect(
-            page.get_by_test_id("key-value-table-row").nth(rowNumber).locator("dt")
+            page.get_by_test_id(
+                "key-value-table-row").nth(rowNumber).locator("dt")
         ).to_contain_text(name)
         expect(
-            page.get_by_test_id("key-value-table-row").nth(rowNumber).locator("dd")
+            page.get_by_test_id(
+                "key-value-table-row").nth(rowNumber).locator("dd")
         ).to_contain_text(value)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_current_fees(page: Page):
     # 6002-MDET-101
-    page.get_by_test_id(market_title_test_id).get_by_text("Current fees").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Current fees").click()
     fields = [
         ["Maker Fee", "10%"],
         ["Infrastructure Fee", "0.05%"],
@@ -54,10 +57,11 @@ def test_market_info_current_fees(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_market_price(page: Page):
     # 6002-MDET-102
-    page.get_by_test_id(market_title_test_id).get_by_text("Market price").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Market price").click()
     fields = [
         ["Mark Price", "107.50"],
         ["Best Bid Price", "101.50"],
@@ -66,10 +70,11 @@ def test_market_info_market_price(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_market_volume(page: Page):
     # 6002-MDET-103
-    page.get_by_test_id(market_title_test_id).get_by_text("Market volume").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Market volume").click()
     fields = [
         ["24 Hour Volume", "-"],
         ["Open Interest", "1"],
@@ -80,17 +85,19 @@ def test_market_info_market_volume(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_insurance_pool(page: Page):
     # 6002-MDET-104
-    page.get_by_test_id(market_title_test_id).get_by_text("Insurance pool").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Insurance pool").click()
     fields = [["Balance", "0.00 tDAI"]]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_key_details(page: Page, vega: VegaServiceNull):
     # 6002-MDET-201
-    page.get_by_test_id(market_title_test_id).get_by_text("Key details").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Key details").click()
     market_id = vega.find_market_id("BTC:DAI_2023")
     short_market_id = market_id[:6] + "…" + market_id[-4:]
     fields = [
@@ -106,7 +113,7 @@ def test_market_info_key_details(page: Page, vega: VegaServiceNull):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_instrument(page: Page):
     # 6002-MDET-202
     page.get_by_test_id(market_title_test_id).get_by_text("Instrument").click()
@@ -121,7 +128,7 @@ def test_market_info_instrument(page: Page):
 
 # @pytest.mark.skip("oracle test to be fixed")
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_oracle(page: Page):
     # 6002-MDET-203
     page.get_by_test_id(market_title_test_id).get_by_text("Oracle").click()
@@ -135,10 +142,11 @@ def test_market_info_oracle(page: Page):
     #     "href", re.compile(rf'(\/oracles\/{vega.find_market_id("BTC:DAI_2023")})')
     # )
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_settlement_asset(page: Page, vega: VegaServiceNull):
     # 6002-MDET-206
-    page.get_by_test_id(market_title_test_id).get_by_text("Settlement asset").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Settlement asset").click()
     tdai_id = vega.find_asset_id("tDAI")
     tdai_id_short = tdai_id[:6] + "…" + tdai_id[-4:]
     fields = [
@@ -155,7 +163,7 @@ def test_market_info_settlement_asset(page: Page, vega: VegaServiceNull):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_metadata(page: Page):
     # 6002-MDET-207
     page.get_by_test_id(market_title_test_id).get_by_text("Metadata").click()
@@ -164,7 +172,7 @@ def test_market_info_metadata(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_risk_model(page: Page):
     # 6002-MDET-208
     page.get_by_test_id(market_title_test_id).get_by_text("Risk model").click()
@@ -175,7 +183,7 @@ def test_market_info_risk_model(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_margin_scaling_factors(page: Page):
     # 6002-MDET-209
     page.get_by_test_id(market_title_test_id).get_by_text(
@@ -183,17 +191,17 @@ def test_market_info_margin_scaling_factors(page: Page):
     ).click()
     fields = [
         ["Linear Slippage Factor", "0.001"],
-        ["Quadratic Slippage Factor", "0"],
         ["Search Level", "1.1"],
         ["Initial Margin", "1.5"],
         ["Collateral Release", "1.7"],
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_risk_factors(page: Page):
     # 6002-MDET-210
-    page.get_by_test_id(market_title_test_id).get_by_text("Risk factors").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Risk factors").click()
     fields = [
         ["Long", "0.05153"],
         ["Short", "0.05422"],
@@ -204,7 +212,7 @@ def test_market_info_risk_factors(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_price_monitoring_bounds(page: Page):
     # 6002-MDET-211
     page.get_by_test_id(market_title_test_id).get_by_text(
@@ -213,27 +221,27 @@ def test_market_info_price_monitoring_bounds(page: Page):
     expect(page.locator("p.col-span-1").nth(0)).to_contain_text(
         "99.9999% probability price bounds"
     )
-    expect(page.locator("p.col-span-1").nth(1)).to_contain_text("Within 86,400 seconds")
+    expect(page.locator("p.col-span-1").nth(1)
+           ).to_contain_text("Within 86,400 seconds")
     fields = [
         ["Highest Price", "138.66685 BTC"],
         ["Lowest Price", "83.11038 BTC"],
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_liquidity_monitoring_parameters(page: Page):
     # 6002-MDET-212
     page.get_by_test_id(market_title_test_id).get_by_text(
         "Liquidity monitoring parameters"
     ).click()
     fields = [
-        ["Triggering Ratio", "0.7"],
         ["Time Window", "3,600"],
         ["Scaling Factor", "1"],
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 # Liquidity resolves to 3 results
 def test_market_info_liquidit(page: Page):
     # 6002-MDET-213
@@ -246,7 +254,7 @@ def test_market_info_liquidit(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_liquidity_price_range(page: Page):
     # 6002-MDET-214
     page.get_by_test_id(market_title_test_id).get_by_text(
@@ -259,19 +267,22 @@ def test_market_info_liquidity_price_range(page: Page):
     ]
     validate_info_section(page, fields)
 
-@pytest.mark.skip("tbd-market-sim")
+
 def test_market_info_proposal(page: Page, vega: VegaServiceNull):
     # 6002-MDET-301
     page.get_by_test_id(market_title_test_id).get_by_text("Proposal").click()
     first_link = (
-        page.get_by_test_id("accordion-content").get_by_test_id("external-link").first
+        page.get_by_test_id(
+            "accordion-content").get_by_test_id("external-link").first
     )
     second_link = (
-        page.get_by_test_id("accordion-content").get_by_test_id("external-link").nth(1)
+        page.get_by_test_id(
+            "accordion-content").get_by_test_id("external-link").nth(1)
     )
     expect(first_link).to_have_text("View governance proposal")
     expect(first_link).to_have_attribute(
-        "href", re.compile(rf'(\/proposals\/{vega.find_market_id("BTC:DAI_2023")})')
+        "href", re.compile(
+            rf'(\/proposals\/{vega.find_market_id("BTC:DAI_2023")})')
     )
     expect(second_link).to_have_text("Propose a change to market")
 
@@ -280,13 +291,14 @@ def test_market_info_proposal(page: Page, vega: VegaServiceNull):
         "href", re.compile(r"(\/proposals\/propose\/update-market)")
     )
 
-    
+
-@pytest.mark.skip("tbd-market-sim")
 def test_market_info_succession_line(page: Page, vega: VegaServiceNull):
-    page.get_by_test_id(market_title_test_id).get_by_text("Succession line").click()
+    page.get_by_test_id(market_title_test_id).get_by_text(
+        "Succession line").click()
     market_id = vega.find_market_id("BTC:DAI_2023")
     succession_line = page.get_by_test_id("succession-line-item")
-    expect(succession_line.get_by_test_id("external-link")).to_have_text("BTC:DAI_2023")
+    expect(succession_line.get_by_test_id(
+        "external-link")).to_have_text("BTC:DAI_2023")
     expect(succession_line.get_by_test_id("external-link")).to_have_attribute(
         "href", re.compile(rf"(\/proposals\/{market_id})")
     )

apps/trading/e2e/tests/successor_market/test_succession_line.py

@@ -8,7 +8,7 @@ from actions.utils import next_epoch
 
 market_banner = "market-banner"
 
-@pytest.mark.skip("tbd")
+
 @pytest.mark.usefixtures("risk_accepted")
 def test_succession_line(vega: VegaServiceNull, page: Page):
     parent_market_id = setup_continuous_market(vega)
@@ -20,12 +20,14 @@ def test_succession_line(vega: VegaServiceNull, page: Page):
     expect(page.get_by_test_id(market_banner)).not_to_be_attached()
 
     successor_name = "successor market name"
-    successor_id = propose_successor(vega, parent_market_id, tdai_id, successor_name)
+    successor_id = propose_successor(
+        vega, parent_market_id, tdai_id, successor_name)
 
     # Check that the banner notifying about the successor proposal is shown
     banner = page.get_by_test_id(market_banner)
     expect(banner).to_be_attached()
-    expect(banner.get_by_text("A successor to this market has been proposed")).to_be_visible()
+    expect(banner.get_by_text(
+        "A successor to this market has been proposed")).to_be_visible()
 
     next_epoch(vega)
 
@@ -45,7 +47,6 @@ def test_succession_line(vega: VegaServiceNull, page: Page):
     # the succession line
     page.reload()
 
-    #tbd issue - 5546
     page.get_by_test_id("Info").click()
 
     page.get_by_role("button", name="Succession line").click()
@@ -78,6 +79,7 @@ def test_succession_line(vega: VegaServiceNull, page: Page):
     page.wait_for_selector('[data-testid="market-banner"]', state="attached")
     expect(banner.get_by_text("This market has been succeeded")).to_be_visible()
 
+
 @pytest.mark.usefixtures("risk_accepted")
 def test_banners(vega: VegaServiceNull, page: Page):
 
@@ -91,9 +93,9 @@ def test_banners(vega: VegaServiceNull, page: Page):
     expect(page.get_by_test_id(market_banner)).not_to_be_attached()
 
     vega.submit_termination_and_settlement_data(
-    settlement_key=GOVERNANCE_WALLET.name,
+        settlement_key=GOVERNANCE_WALLET.name,
-    settlement_price=100,
+        settlement_price=100,
-    market_id=parent_market_id,
+        market_id=parent_market_id,
     )
 
     successor_name = "successor market name"
@@ -108,7 +110,7 @@ def test_banners(vega: VegaServiceNull, page: Page):
     # Check that the banner notifying about the successor proposal and market has been settled are shown still after reload
     page.reload()
     expect(banner.get_by_text(banner_successor_text)).to_be_visible()
-    expect(banner.get_by_text("1/2")).to_be_visible() 
+    expect(banner.get_by_text("1/2")).to_be_visible()
     # Check that the banner notifying about the successor proposal is not visible after close those banners
     banner.get_by_test_id("icon-cross").click()
     expect(banner.get_by_text("This market has been settled")).to_be_visible()
@@ -119,7 +121,8 @@ def test_banners(vega: VegaServiceNull, page: Page):
     expect(page.get_by_test_id(market_banner)).not_to_be_attached()
     page.reload()
     expect(banner).to_be_attached()
-    expect(banner.get_by_text(banner_successor_text)).to_be_visible() 
+    expect(banner.get_by_text(banner_successor_text)).to_be_visible()
+
 
 def propose_successor(
     vega: VegaServiceNull, parent_market_id, tdai_id, market_name
@@ -137,6 +140,7 @@ def propose_successor(
     )
     return market_id
 
+
 def provide_successor_liquidity(
     vega: VegaServiceNull, market_id
 ):

libs/assets/src/lib/assets-data-provider.ts

@@ -29,6 +29,7 @@ export const assetsProvider = makeDataProvider<
 >({
   query: AssetsDocument,
   getData,
+  errorPolicy: 'all',
 });
 
 export const assetsMapProvider = makeDerivedDataProvider<

libs/data-provider/src/generic-data-provider.spec.ts

@@ -22,11 +22,9 @@ import {
   type QueryOptions,
   type ApolloClient,
 } from '@apollo/client';
-import { ApolloError } from '@apollo/client';
+import { type ApolloError } from '@apollo/client';
-import type { GraphQLErrors } from '@apollo/client/errors';
 import { GraphQLError } from 'graphql';
 import { type Subscription, type Observable } from 'zen-observable-ts';
-import { waitFor } from '@testing-library/react';
 
 type Item = {
   cursor: string;
@@ -117,24 +115,6 @@ const paginatedSubscribe = makeDataProvider<
   },
 });
 
-const mockErrorPolicyGuard: (errors: GraphQLErrors) => boolean = jest
-  .fn()
-  .mockImplementation(() => true);
-const errorGuardedSubscribe = makeDataProvider<
-  QueryData,
-  Data,
-  SubscriptionData,
-  Delta,
-  Variables
->({
-  query,
-  subscriptionQuery,
-  update,
-  getData,
-  getDelta,
-  errorPolicyGuard: mockErrorPolicyGuard,
-});
-
 const derivedSubscribe = makeDerivedDataProvider(
   [paginatedSubscribe, subscribe],
   combineData,
@@ -404,34 +384,6 @@ describe('data provider', () => {
 
     subscription.unsubscribe();
   });
-
-  it('should retry with ignore error policy if errorPolicyGuard returns true', async () => {
-    const subscription = errorGuardedSubscribe(callback, client, variables);
-    const graphQLError = new GraphQLError(
-      '',
-      undefined,
-      undefined,
-      undefined,
-      ['market', 'data'],
-      undefined,
-      {
-        type: 'Internal',
-      }
-    );
-    const graphQLErrors = [graphQLError];
-    const error = new ApolloError({ graphQLErrors });
-
-    await rejectQuery(error);
-    const data = generateData(0, 5);
-    await resolveQuery({
-      data,
-    });
-    expect(mockErrorPolicyGuard).toHaveBeenNthCalledWith(1, graphQLErrors);
-    await waitFor(() =>
-      expect(getData).toHaveBeenCalledWith({ data }, variables)
-    );
-    subscription.unsubscribe();
-  });
 });
 
 describe('derived data provider', () => {

libs/data-provider/src/generic-data-provider.ts

@@ -9,7 +9,6 @@ import type {
   ApolloQueryResult,
   QueryOptions,
 } from '@apollo/client';
-import type { GraphQLErrors } from '@apollo/client/errors';
 import type { Subscription } from 'zen-observable-ts';
 import isEqualWith from 'lodash/isEqualWith';
 import { isNotFoundGraphQLError } from './helpers';
@@ -161,7 +160,7 @@ interface DataProviderParams<
   resetDelay?: number;
   pollInterval?: number;
   additionalContext?: Record<string, unknown>;
-  errorPolicyGuard?: (graphqlErrors: GraphQLErrors) => boolean;
+  errorPolicy?: ErrorPolicy;
   getQueryVariables?: (variables: Variables) => QueryVariables;
   getSubscriptionVariables?: (
     variables: Variables
@@ -176,7 +175,7 @@ interface DataProviderParams<
  * @param fetchPolicy
  * @param resetDelay
  * @param additionalContext add property to the context of the query, ie. 'isEnlargedTimeout'
- * @param errorPolicyGuard indicate which gql errors can be tolerate
+ * @param errorPolicy Apollos error policy, will be used when querying
  * @returns subscribe function
  */
 function makeDataProviderInternal<
@@ -197,7 +196,7 @@ function makeDataProviderInternal<
   fetchPolicy,
   resetDelay,
   additionalContext,
-  errorPolicyGuard,
+  errorPolicy = 'none',
   getQueryVariables,
   getSubscriptionVariables,
   pollInterval,
@@ -331,20 +330,10 @@ function makeDataProviderInternal<
   const callQuery = (
     pagination?: Pagination,
     policy?: ErrorPolicy
-  ): Promise<ApolloQueryResult<QueryData>> =>
+  ): Promise<ApolloQueryResult<QueryData>> => {
-    client
+    const options = getQueryOptions(pagination, policy);
-      .query<QueryData>(getQueryOptions(pagination, policy))
+    return client.query<QueryData>(options);
-      .catch((err) => {
+  };
-        if (
-          err.graphQLErrors &&
-          errorPolicyGuard &&
-          errorPolicyGuard(err.graphQLErrors)
-        ) {
-          return callQuery(pagination, 'ignore');
-        } else {
-          throw err;
-        }
-      });
 
   const load = async () => {
     if (!pagination) {
@@ -364,7 +353,7 @@ function makeDataProviderInternal<
       }
     }
 
-    const res = await callQuery(paginationVariables);
+    const res = await callQuery(paginationVariables, errorPolicy);
 
     const insertionData = getData(res.data, variables);
     const insertionPageInfo = pagination.getPageInfo(res.data);
@@ -417,12 +406,14 @@ function makeDataProviderInternal<
     const paginationVariables = pagination
       ? { first: pagination.first }
       : undefined;
+
     if (pollInterval) {
-      callWatchQuery();
+      callWatchQuery(paginationVariables, errorPolicy);
       return;
     }
+
     try {
-      onNext(await callQuery(paginationVariables));
+      onNext(await callQuery(paginationVariables, errorPolicy));
     } catch (e) {
       onError(e as Error);
     } finally {

libs/data-provider/src/helpers.ts

@@ -27,10 +27,3 @@ const hasNotFoundGraphQLErrors = (errors: GraphQLErrors, path?: string[]) => {
       (!path || path.every((item, i) => item === e?.path?.[i]))
   );
 };
-
-export const marketDataErrorPolicyGuard = (errors: GraphQLErrors) =>
-  errors.every(
-    (e) =>
-      e.message.match(/no market data for market:/i) ||
-      e.message.match(/Conditions list is empty/)
-  );

libs/markets/src/lib/components/market-info/market-info-data-provider.ts

@@ -1,7 +1,6 @@
 import {
   makeDataProvider,
   makeDerivedDataProvider,
-  marketDataErrorPolicyGuard,
 } from '@vegaprotocol/data-provider';
 import {
   MarketInfoDocument,
@@ -33,7 +32,7 @@ export const marketInfoProvider = makeDataProvider<
 >({
   query: MarketInfoDocument,
   getData,
-  errorPolicyGuard: marketDataErrorPolicyGuard,
+  errorPolicy: 'all',
   pollInterval: 5000,
 });
 

libs/markets/src/lib/markets-data-provider.ts

@@ -1,4 +1,3 @@
-import { marketDataErrorPolicyGuard } from '@vegaprotocol/data-provider';
 import { makeDataProvider } from '@vegaprotocol/data-provider';
 import {
   MarketsDataDocument,
@@ -54,7 +53,7 @@ export const marketsDataProvider = makeDataProvider<
 >({
   query: MarketsDataDocument,
   getData,
-  errorPolicyGuard: marketDataErrorPolicyGuard,
+  errorPolicy: 'all',
 });
 
 type Variables = { marketIds: string[] };
@@ -73,7 +72,7 @@ export const marketsLiveDataProvider = makeDataProvider<
   getData,
   getDelta,
   update,
-  errorPolicyGuard: marketDataErrorPolicyGuard,
+  errorPolicy: 'all',
   getQueryVariables: () => ({}),
   getSubscriptionVariables: ({ marketIds }: Variables) =>
     marketIds.map((marketId) => ({ marketId })),

libs/markets/src/lib/markets-provider.ts

@@ -2,7 +2,6 @@ import { useYesterday } from '@vegaprotocol/react-helpers';
 import {
   makeDataProvider,
   makeDerivedDataProvider,
-  marketDataErrorPolicyGuard,
   useDataProvider,
 } from '@vegaprotocol/data-provider';
 import {
@@ -45,7 +44,7 @@ export const marketsProvider = makeDataProvider<
   query: MarketsDocument,
   getData,
   fetchPolicy: 'cache-first',
-  errorPolicyGuard: marketDataErrorPolicyGuard,
+  errorPolicy: 'all',
 });
 
 export const marketsMapProvider = makeDerivedDataProvider<

libs/proposals/src/lib/proposals-data-provider/proposals-data-provider.tsx

@@ -39,8 +39,7 @@ export const proposalsDataProvider = makeDataProvider<
    *
    * GQL Path: `terms.change.instrument.futureProduct.settlementAsset`
    */
-  errorPolicyGuard: (errors) =>
+  errorPolicy: 'all',
-    errors.every((e) => e.message.match(/failed to get asset for ID/)),
 });
 
 const ProposalTypeMap: Record<