From 4dd63da62b21c3a2585bea9eb84ea9145192bd8a Mon Sep 17 00:00:00 2001 From: Maciek Date: Thu, 9 Feb 2023 14:26:23 +0100 Subject: [PATCH] chore(trading): handle positions with market data gql errors (#2884) --- .../src/integration/trading-positions.cy.ts | 44 +++++++++++ libs/cypress/src/lib/mock-gql.ts | 8 +- .../src/lib/markets-data-provider.ts | 6 +- .../src/lib/positions-data-providers.ts | 77 +++++++++++-------- libs/positions/src/lib/positions-table.tsx | 18 ++--- libs/react-helpers/src/lib/apollo-client.ts | 17 +++- .../src/lib/generic-data-provider.spec.ts | 50 +++++++++++- .../src/lib/generic-data-provider.ts | 62 ++++++++++----- 8 files changed, 212 insertions(+), 70 deletions(-) diff --git a/apps/trading-e2e/src/integration/trading-positions.cy.ts b/apps/trading-e2e/src/integration/trading-positions.cy.ts index 48f4b0166..d3af225f8 100644 --- a/apps/trading-e2e/src/integration/trading-positions.cy.ts +++ b/apps/trading-e2e/src/integration/trading-positions.cy.ts @@ -1,3 +1,6 @@ +import { aliasGQLQuery } from '@vegaprotocol/cypress'; +import { marketsDataQuery } from '@vegaprotocol/mock'; + beforeEach(() => { cy.mockTradingPage(); cy.mockSubscription(); @@ -17,6 +20,47 @@ describe('positions', { tags: '@smoke' }, () => { validatePositionsDisplayed(); }); + it('renders position among some graphql errors', () => { + const errors = [ + { + message: 'no market data for market: market-2', + path: ['market', 'data'], + extensions: { + code: 13, + type: 'Internal', + }, + }, + ]; + const marketData = marketsDataQuery(); + const edges = marketData.marketsConnection?.edges.map((market) => { + const replace = + market.node.data?.market.id === 'market-2' ? null : market.node.data; + return { ...market, node: { ...market.node, data: replace } }; + }); + const overrides = { + ...marketData, + marketsConnection: { ...marketData.marketsConnection, edges }, + }; + cy.mockGQL((req) => { + aliasGQLQuery(req, 'MarketsData', overrides, errors); + }); + cy.visit('/#/markets/market-0'); + const emptyCells = [ + 'notional', + 'markPrice', + 'liquidationPrice', + 'currentLeverage', + 'averageEntryPrice', + ]; + cy.getByTestId('tab-positions').within(() => { + cy.get('[row-id="market-2"]').within(() => { + emptyCells.forEach((cell) => { + cy.get(`[col-id="${cell}"]`).should('contain.text', '-'); + }); + }); + }); + }); + function validatePositionsDisplayed() { cy.getByTestId('tab-positions').should('be.visible'); cy.getByTestId('tab-positions').within(() => { diff --git a/libs/cypress/src/lib/mock-gql.ts b/libs/cypress/src/lib/mock-gql.ts index 90e2b80a0..8ae3544b8 100644 --- a/libs/cypress/src/lib/mock-gql.ts +++ b/libs/cypress/src/lib/mock-gql.ts @@ -1,3 +1,4 @@ +import type { GraphQLError } from 'graphql'; import type { RouteHandler } from 'cypress/types/net-stubbing'; import type { CyHttpMessages } from 'cypress/types/net-stubbing'; @@ -30,14 +31,15 @@ export const aliasGQLQuery = ( req: CyHttpMessages.IncomingHttpRequest, operationName: string, // eslint-disable-next-line @typescript-eslint/no-explicit-any - data?: any + data?: any, + errors?: Partial[] ) => { if (hasOperationName(req, operationName)) { req.alias = operationName; - if (data !== undefined) { + if (data !== undefined || errors !== undefined) { req.reply({ statusCode: 200, - body: { data }, + body: { ...(data && { data }), ...(errors && { errors }) }, }); } } diff --git a/libs/market-list/src/lib/markets-data-provider.ts b/libs/market-list/src/lib/markets-data-provider.ts index 029b67235..09f88099c 100644 --- a/libs/market-list/src/lib/markets-data-provider.ts +++ b/libs/market-list/src/lib/markets-data-provider.ts @@ -1,4 +1,7 @@ -import { makeDataProvider } from '@vegaprotocol/react-helpers'; +import { + makeDataProvider, + marketDataErrorPolicyGuard, +} from '@vegaprotocol/react-helpers'; import type { MarketsDataQuery } from './__generated__/markets-data'; import { MarketsDataDocument } from './__generated__/markets-data'; import type { MarketData } from './market-data-provider'; @@ -16,4 +19,5 @@ export const marketsDataProvider = makeDataProvider< >({ query: MarketsDataDocument, getData, + errorPolicyGuard: marketDataErrorPolicyGuard, }); diff --git a/libs/positions/src/lib/positions-data-providers.ts b/libs/positions/src/lib/positions-data-providers.ts index 423af9504..99aaa9efb 100644 --- a/libs/positions/src/lib/positions-data-providers.ts +++ b/libs/positions/src/lib/positions-data-providers.ts @@ -45,22 +45,22 @@ export interface Position { averageEntryPrice: string; marginAccountBalance: string; capitalUtilisation: number; - currentLeverage: number; + currentLeverage: number | undefined; decimals: number; marketDecimalPlaces: number; positionDecimalPlaces: number; totalBalance: string; assetSymbol: string; - liquidationPrice: string; + liquidationPrice: string | undefined; lowMarginLevel: boolean; marketId: string; marketTradingMode: Schema.MarketTradingMode; - markPrice: string; - notional: string; + markPrice: string | undefined; + notional: string | undefined; openVolume: string; realisedPNL: string; unrealisedPNL: string; - searchPrice: string; + searchPrice: string | undefined; updatedAt: string | null; } @@ -84,7 +84,7 @@ export const getMetrics = ( const marginAccount = accounts?.find((account) => { return account.market?.id === market?.id; }); - if (!marginAccount || !marginLevel || !market || !marketData) { + if (!marginAccount || !marginLevel || !market) { return; } const generalAccount = accounts?.find( @@ -102,15 +102,22 @@ export const getMetrics = ( generalAccount?.balance ?? 0, decimals ); - const markPrice = toBigNum(marketData.markPrice, marketDecimalPlaces); - const notional = ( - openVolume.isGreaterThan(0) ? openVolume : openVolume.multipliedBy(-1) - ).multipliedBy(markPrice); + const markPrice = marketData + ? toBigNum(marketData.markPrice, marketDecimalPlaces) + : undefined; + const notional = markPrice + ? (openVolume.isGreaterThan(0) + ? openVolume + : openVolume.multipliedBy(-1) + ).multipliedBy(markPrice) + : undefined; const totalBalance = marginAccountBalance.plus(generalAccountBalance); - const currentLeverage = totalBalance.isEqualTo(0) - ? new BigNumber(0) - : notional.dividedBy(totalBalance); + const currentLeverage = notional + ? totalBalance.isEqualTo(0) + ? new BigNumber(0) + : notional.dividedBy(totalBalance) + : undefined; const capitalUtilisation = totalBalance.isEqualTo(0) ? new BigNumber(0) : marginAccountBalance.dividedBy(totalBalance).multipliedBy(100); @@ -119,19 +126,23 @@ export const getMetrics = ( const marginSearch = toBigNum(marginLevel.searchLevel, decimals); const marginInitial = toBigNum(marginLevel.initialLevel, decimals); - const searchPrice = marginSearch - .minus(marginAccountBalance) - .dividedBy(openVolume) - .plus(markPrice); + const searchPrice = markPrice + ? marginSearch + .minus(marginAccountBalance) + .dividedBy(openVolume) + .plus(markPrice) + : undefined; - const liquidationPrice = BigNumber.maximum( - 0, - marginMaintenance - .minus(marginAccountBalance) - .minus(generalAccountBalance) - .dividedBy(openVolume) - .plus(markPrice) - ); + const liquidationPrice = markPrice + ? BigNumber.maximum( + 0, + marginMaintenance + .minus(marginAccountBalance) + .minus(generalAccountBalance) + .dividedBy(openVolume) + .plus(markPrice) + ) + : undefined; const lowMarginLevel = marginAccountBalance.isLessThan( @@ -143,7 +154,7 @@ export const getMetrics = ( averageEntryPrice: position.averageEntryPrice, marginAccountBalance: marginAccount.balance, capitalUtilisation: Math.round(capitalUtilisation.toNumber()), - currentLeverage: currentLeverage.toNumber(), + currentLeverage: currentLeverage ? currentLeverage.toNumber() : undefined, marketDecimalPlaces, positionDecimalPlaces, decimals, @@ -152,18 +163,20 @@ export const getMetrics = ( totalBalance: totalBalance.multipliedBy(10 ** decimals).toFixed(), lowMarginLevel, liquidationPrice: liquidationPrice - .multipliedBy(10 ** marketDecimalPlaces) - .toFixed(0), + ? liquidationPrice.multipliedBy(10 ** marketDecimalPlaces).toFixed(0) + : undefined, marketId: market.id, marketTradingMode: market.tradingMode, - markPrice: marketData.markPrice, - notional: notional.multipliedBy(10 ** marketDecimalPlaces).toFixed(0), + markPrice: marketData ? marketData.markPrice : undefined, + notional: notional + ? notional.multipliedBy(10 ** marketDecimalPlaces).toFixed(0) + : undefined, openVolume: position.openVolume, realisedPNL: position.realisedPNL, unrealisedPNL: position.unrealisedPNL, searchPrice: searchPrice - .multipliedBy(10 ** marketDecimalPlaces) - .toFixed(0), + ? searchPrice.multipliedBy(10 ** marketDecimalPlaces).toFixed(0) + : undefined, updatedAt: position.updatedAt || null, }); }); diff --git a/libs/positions/src/lib/positions-table.tsx b/libs/positions/src/lib/positions-table.tsx index bfb6b5797..b9311c44b 100644 --- a/libs/positions/src/lib/positions-table.tsx +++ b/libs/positions/src/lib/positions-table.tsx @@ -49,7 +49,7 @@ export const AmountCell = ({ valueFormatted }: AmountCellProps) => { } const { openVolume, positionDecimalPlaces, marketDecimalPlaces, notional } = valueFormatted; - return valueFormatted ? ( + return valueFormatted && notional ? (
( valueGetter={({ data, }: VegaValueGetterParams) => { - return data?.notional === undefined + return !data?.notional ? undefined - : toBigNum(data?.notional, data.marketDecimalPlaces).toNumber(); + : toBigNum(data.notional, data.marketDecimalPlaces).toNumber(); }} valueFormatter={({ data, }: VegaValueFormatterParams) => { - return !data - ? undefined + return !data || !data.notional + ? '-' : addDecimalsFormatNumber( data.notional, data.marketDecimalPlaces @@ -173,6 +173,7 @@ export const PositionsTable = forwardRef( data, }: VegaValueGetterParams) => { return !data || + !data.markPrice || data.marketTradingMode === Schema.MarketTradingMode.TRADING_MODE_OPENING_AUCTION ? undefined @@ -180,14 +181,14 @@ export const PositionsTable = forwardRef( }} valueFormatter={({ data, - node, }: VegaValueFormatterParams) => { if (!data) { return undefined; } if ( + !data.markPrice || data.marketTradingMode === - Schema.MarketTradingMode.TRADING_MODE_OPENING_AUCTION + Schema.MarketTradingMode.TRADING_MODE_OPENING_AUCTION ) { return '-'; } @@ -220,7 +221,6 @@ export const PositionsTable = forwardRef( }} valueFormatter={({ data, - node, }: VegaValueFormatterParams): | string | undefined => { @@ -258,7 +258,7 @@ export const PositionsTable = forwardRef( }: VegaValueFormatterParams): | string | undefined => { - if (!data) { + if (!data || data?.liquidationPrice === undefined) { return undefined; } return addDecimalsFormatNumber( diff --git a/libs/react-helpers/src/lib/apollo-client.ts b/libs/react-helpers/src/lib/apollo-client.ts index 20bb254b8..f5457f197 100644 --- a/libs/react-helpers/src/lib/apollo-client.ts +++ b/libs/react-helpers/src/lib/apollo-client.ts @@ -2,6 +2,7 @@ import type { ApolloError } from '@apollo/client'; import type { GraphQLErrors } from '@apollo/client/errors'; const NOT_FOUND = 'NotFound'; +const INTERNAL = 'Internal'; const isApolloGraphQLError = ( error: ApolloError | Error | undefined @@ -9,21 +10,31 @@ const isApolloGraphQLError = ( return !!error && !!(error as ApolloError).graphQLErrors; }; -const hasNotFoundGraphQLErrors = (errors: GraphQLErrors, path?: string) => { +const hasNotFoundGraphQLErrors = (errors: GraphQLErrors, path?: string[]) => { return errors.some( (e) => e.extensions && e.extensions['type'] === NOT_FOUND && - (!path || e.path?.[0] === path) + (!path || path.every((item, i) => item === e?.path?.[i])) ); }; export const isNotFoundGraphQLError = ( error: Error | ApolloError | undefined, - path?: string + path?: string[] ) => { return ( isApolloGraphQLError(error) && hasNotFoundGraphQLErrors(error.graphQLErrors, path) ); }; + +export const marketDataErrorPolicyGuard = (errors: GraphQLErrors) => { + const path = ['market', 'data']; + return errors.every( + (e) => + e.extensions && + e.extensions['type'] === INTERNAL && + (!path || path.every((item, i) => item === e?.path?.[i])) + ); +}; diff --git a/libs/react-helpers/src/lib/generic-data-provider.spec.ts b/libs/react-helpers/src/lib/generic-data-provider.spec.ts index 96c6f9e58..7a00ac42e 100644 --- a/libs/react-helpers/src/lib/generic-data-provider.spec.ts +++ b/libs/react-helpers/src/lib/generic-data-provider.spec.ts @@ -19,11 +19,12 @@ import type { OperationVariables, ApolloQueryResult, QueryOptions, - ApolloError, } from '@apollo/client'; +import { ApolloError } from '@apollo/client'; +import type { GraphQLErrors } from '@apollo/client/errors'; import { GraphQLError } from 'graphql'; - import type { Subscription, Observable } from 'zen-observable-ts'; +import { waitFor } from '@testing-library/react'; type Item = { cursor: string; @@ -108,6 +109,23 @@ const paginatedSubscribe = makeDataProvider< }, }); +const mockErrorPolicyGuard: (errors: GraphQLErrors) => boolean = jest + .fn() + .mockImplementation(() => true); +const errorGuardedSubscribe = makeDataProvider< + QueryData, + Data, + SubscriptionData, + Delta +>({ + query, + subscriptionQuery, + update, + getData, + getDelta, + errorPolicyGuard: mockErrorPolicyGuard, +}); + const derivedSubscribe = makeDerivedDataProvider( [paginatedSubscribe, subscribe], combineData, @@ -537,6 +555,34 @@ describe('data provider', () => { expect(lastCallbackArgs[0].totalCount).toBe(100); subscription.unsubscribe(); }); + + it('errorPolicyGuard should work properly', async () => { + const subscription = errorGuardedSubscribe(callback, client); + 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 }, undefined) + ); + subscription.unsubscribe(); + }); }); describe('derived data provider', () => { diff --git a/libs/react-helpers/src/lib/generic-data-provider.ts b/libs/react-helpers/src/lib/generic-data-provider.ts index 8813c6c8d..4118399c7 100644 --- a/libs/react-helpers/src/lib/generic-data-provider.ts +++ b/libs/react-helpers/src/lib/generic-data-provider.ts @@ -5,7 +5,10 @@ import type { OperationVariables, TypedDocumentNode, FetchResult, + ErrorPolicy, + ApolloQueryResult, } from '@apollo/client'; +import type { GraphQLErrors } from '@apollo/client/errors'; import type { Subscription } from 'zen-observable-ts'; import isEqual from 'lodash/isEqual'; import { isNotFoundGraphQLError } from './apollo-client'; @@ -178,6 +181,7 @@ interface DataProviderParams< fetchPolicy?: FetchPolicy; resetDelay?: number; additionalContext?: Record; + errorPolicyGuard?: (graphqlErrors: GraphQLErrors) => boolean; } /** @@ -186,6 +190,9 @@ interface DataProviderParams< * @param getData transforms received query data to format that will be stored in data provider * @param getDelta transforms delta data to format that will be stored in data provider * @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 * @returns subscribe function */ function makeDataProviderInternal< @@ -204,6 +211,7 @@ function makeDataProviderInternal< fetchPolicy, resetDelay, additionalContext, + errorPolicyGuard, }: DataProviderParams< QueryData, Data, @@ -248,6 +256,30 @@ function makeDataProviderInternal< callbacks.forEach((callback) => notify(callback, updateData)); }; + const call = ( + pagination?: Pagination, + policy?: ErrorPolicy + ): Promise> => + client + .query({ + query, + variables: { ...variables, ...(pagination && { pagination }) }, + fetchPolicy: fetchPolicy || 'no-cache', + context: additionalContext, + errorPolicy: policy || 'none', + }) + .catch((err) => { + if ( + err.graphQLErrors && + errorPolicyGuard && + errorPolicyGuard(err.graphQLErrors) + ) { + return call(pagination, 'ignore'); + } else { + throw err; + } + }); + const load = async (start?: number, end?: number) => { if (!pagination) { return Promise.reject(); @@ -276,15 +308,9 @@ function makeDataProviderInternal< } else if (!pageInfo?.hasNextPage) { return null; } - const res = await client.query({ - query, - variables: { - ...variables, - pagination: paginationVariables, - }, - fetchPolicy: fetchPolicy || 'no-cache', - context: additionalContext, - }); + + const res = await call(paginationVariables); + const insertionData = getData(res.data, variables); const insertionPageInfo = pagination.getPageInfo(res.data); ({ data, totalCount } = pagination.append( @@ -313,15 +339,11 @@ function makeDataProviderInternal< if (!client) { return; } + const paginationVariables = pagination + ? { first: pagination.first } + : undefined; try { - const res = await client.query({ - query, - variables: pagination - ? { ...variables, pagination: { first: pagination.first } } - : variables, - fetchPolicy: fetchPolicy || 'no-cache', - context: additionalContext, - }); + const res = await call(paginationVariables); data = getData(res.data, variables); if (data && pagination) { if (!(data instanceof Array)) { @@ -355,7 +377,7 @@ function makeDataProviderInternal< } loaded = true; } catch (e) { - if (isNotFoundGraphQLError(e as Error, 'party')) { + if (isNotFoundGraphQLError(e as Error, ['party'])) { data = getData(null, variables); loaded = true; return; @@ -495,7 +517,7 @@ const memoize = < Delta, Variables extends OperationVariables = OperationVariables >( - fn: (variables?: Variables) => Subscribe + fn: () => Subscribe ) => { const cache: { subscribe: Subscribe; @@ -506,7 +528,7 @@ const memoize = < if (cached) { return cached.subscribe; } - const subscribe = fn(variables); + const subscribe = fn(); cache.push({ subscribe, variables }); return subscribe; };