mirror of
https://github.com/SigNoz/signoz.git
synced 2026-08-30 08:10:29 +01:00
098448330d
<!--A few plain bullets saying what changed and why, for a reviewer
skimming it - not a wall of text, not a restatement of the diff, not
generated boilerplate.-->
#### Description
Fixes an unhandled promise rejection on the APM service detail page —
opening a chart's drilldown menu could throw `syntax errors in
expression: [line 1:48 missing {BOOL, NUMBER, QUOTED_TEXT, KEY} at
']']`.
The chain:
- The overview's top-level-operations query keys on `minTime`/`maxTime`
with no `keepPreviousData`, so every time-range change blanks the list.
The widgets below are then rebuilt with `service.name in ['<service>']
AND operation in []`.
- `valueList` in the filter grammar needs at least one value, so `in []`
is a hard parse error. The panel itself is guarded (`isQueryEnabled`
requires a non-empty list); the drilldown is not.
- The drilldown menu resolves the widget query through
`/substitute_vars` on every click — on this page there are no dashboard
variables at all, so it is pure overhead — and the 400 landed on a
floating promise with no rejection handler.
What changed:
- `useBaseAggregateOptions` catches the failure, falls back to the
unresolved query (already its initial state) and shows the same "Unable
to resolve variables" toast `useNavigateToExplorer` uses. `oxlint` was
already flagging this line under `no-floating-promises`; that warning is
gone.
- `useResolveQuery` short-circuits when there are no variables to
substitute, so APM / Celery / API monitoring drilldowns stop making the
call at all.
- The overview keeps its previous operations list across a time-range
change, so the widget queries are never built with an empty list — which
also stopped the bad filter riding into the explorer URL the drilldown
opens.
#### Issues Closed
Closes https://github.com/SigNoz/pulse-pod/issues/278
<!--Anything reviewers should keep in mind while reviewing -->
#### Additional Information
- Skipping the round-trip doesn't lose the filter: both consumers of the
resolved query rebuild `filter.expression` from `filters.items`
themselves (`getViewQuery`, `useGetCompositeQueryParam`), and the APM
query factory never sets `filter.expression` to begin with.
- `keepPreviousData` is safe across service navigation —
`topLevelOperations[servicename]` already returns `[]` for a mismatched
service, and `isQueryEnabled` still guards that case.
- Deliberately left out: dropping empty `IN []` items globally in
`convertFiltersToExpression`. It would silence a wider class of 400s but
flips the semantics — `IN []` means "match nothing", dropping the clause
means "match everything" — and there is an existing test asserting
today's behaviour. Happy to do it separately as a match-nothing rewrite
if reviewers want the broader guard.
- Sentry: SIGNOZ-UI-5JV.
85 lines
2.8 KiB
TypeScript
85 lines
2.8 KiB
TypeScript
import { useCallback } from 'react';
|
|
import { useMutation } from 'react-query';
|
|
// eslint-disable-next-line no-restricted-imports
|
|
import { useSelector } from 'react-redux';
|
|
import { isEmpty } from 'lodash-es';
|
|
import { getSubstituteVars } from 'api/dashboard/substitute_vars';
|
|
import { prepareQueryRangePayloadV5 } from 'api/v5/v5';
|
|
import { PANEL_TYPES } from 'constants/queryBuilder';
|
|
import { timePreferenceType } from 'container/NewWidget/RightContainer/timeItems';
|
|
import { useDashboardVariablesByType } from 'hooks/dashboard/useDashboardVariablesByType';
|
|
import { getDashboardVariables } from 'lib/dashboardVariables/getDashboardVariables';
|
|
import { mapQueryDataFromApi } from 'lib/newQueryBuilder/queryBuilderMappers/mapQueryDataFromApi';
|
|
import { AppState } from 'store/reducers';
|
|
import { Query } from 'types/api/queryBuilder/queryBuilderData';
|
|
import { GlobalReducer } from 'types/reducer/globalTime';
|
|
import { getGraphType } from 'utils/getGraphType';
|
|
|
|
interface UseUpdatedQueryOptions {
|
|
widgetConfig: {
|
|
query: Query;
|
|
panelTypes: PANEL_TYPES;
|
|
timePreferance: timePreferenceType;
|
|
};
|
|
dashboardData?: any;
|
|
}
|
|
|
|
interface UseUpdatedQueryResult {
|
|
getUpdatedQuery: (options: UseUpdatedQueryOptions) => Promise<Query>;
|
|
isLoading: boolean;
|
|
}
|
|
|
|
function useUpdatedQuery(): UseUpdatedQueryResult {
|
|
const { selectedTime: globalSelectedInterval } = useSelector<
|
|
AppState,
|
|
GlobalReducer
|
|
>((state) => state.globalTime);
|
|
|
|
const queryRangeMutation = useMutation(getSubstituteVars);
|
|
|
|
const dashboardDynamicVariables = useDashboardVariablesByType(
|
|
'DYNAMIC',
|
|
'values',
|
|
);
|
|
|
|
const getUpdatedQuery = useCallback(
|
|
async ({
|
|
widgetConfig,
|
|
dashboardData,
|
|
}: UseUpdatedQueryOptions): Promise<Query> => {
|
|
const variables = getDashboardVariables(dashboardData?.data?.variables);
|
|
|
|
// `/substitute_vars` only rewrites `$variable` references, so on surfaces with no
|
|
// dashboard behind them (APM, Celery, API monitoring) the round-trip is a no-op.
|
|
if (isEmpty(variables) && isEmpty(dashboardDynamicVariables)) {
|
|
return widgetConfig.query;
|
|
}
|
|
|
|
// Prepare query payload with resolved variables
|
|
const { queryPayload } = prepareQueryRangePayloadV5({
|
|
query: widgetConfig.query,
|
|
graphType: getGraphType(widgetConfig.panelTypes),
|
|
selectedTime: widgetConfig.timePreferance,
|
|
globalSelectedInterval,
|
|
variables,
|
|
originalGraphType: widgetConfig.panelTypes,
|
|
dynamicVariables: dashboardDynamicVariables,
|
|
});
|
|
|
|
// Execute query and process results
|
|
const queryResult = await queryRangeMutation.mutateAsync(queryPayload);
|
|
|
|
// Map query data from API response
|
|
return mapQueryDataFromApi(queryResult.data.compositeQuery);
|
|
},
|
|
[dashboardDynamicVariables, globalSelectedInterval, queryRangeMutation],
|
|
);
|
|
|
|
return {
|
|
getUpdatedQuery,
|
|
isLoading: queryRangeMutation.isLoading,
|
|
};
|
|
}
|
|
|
|
export default useUpdatedQuery;
|