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

[BUG] no fallback when configured tiebreaker fields are invalid or unsortable and break view surrounding documents function #8746

Open
ananzh opened this issue Oct 29, 2024 · 0 comments
Labels
bug Something isn't working discover for discover reinvent

Comments

@ananzh
Copy link
Member

ananzh commented Oct 29, 2024

Describe the bug
The system allows setting custom tiebreaker fields in advanced settings ([currency,category]). When these fields don't exist or aren't sortable in a particular index pattern, the system fails instead of falling back to a reliable default like _doc. This creates a poor user experience where the context view fails completely without clear indication of why or how to fix it.

Screenshot 2024-10-27 at 4 12 08 PM

So tieBreakerField here appears to be a secondary sorting field used to ensure consistent ordering when multiple documents have the same timestamp value.

export function useQueryActions(anchorId: string, indexPattern: IndexPattern) {
  const { services } = useOpenSearchDashboards<DiscoverServices>();
  const { data, uiSettings, toastNotifications } = services;
  const searchSource = useMemo(() => {
    return data.search.searchSource.createEmpty();
  }, [data.search.searchSource]);
  const tieBreakerField = useMemo(
    () => getFirstSortableField(indexPattern, uiSettings.get(CONTEXT_TIE_BREAKER_FIELDS_SETTING)),
    [uiSettings, indexPattern]
  );
  const [contextQueryState, setContextQueryState] = useState<ContextQueryState>(initialState);

  const fetchAnchorRow = useCallback(async () => {
    if (!tieBreakerField) {
      setContextQueryState((prevState) => ({
        ...prevState,
        anchorStatus: {
          value: LOADING_STATUS.FAILED,
          reason: FAILURE_REASONS.INVALID_TIEBREAKER,
        },
      }));
      return;
    }

    try {
      setContextQueryState((prevState) => ({
        ...prevState,
        anchorStatus: { value: LOADING_STATUS.LOADING },
      }));
      const sort = [
        { [indexPattern.timeFieldName!]: SortDirection.desc },
        { [tieBreakerField]: SortDirection.desc },
      ];
      const fetchAnchorResult = await fetchAnchor(anchorId, indexPattern, searchSource, sort);
      setContextQueryState((prevState) => ({
        ...prevState,
        anchor: fetchAnchorResult,
        anchorStatus: { value: LOADING_STATUS.LOADED },
      }));
      return fetchAnchorResult;
    } catch (error) {
      setContextQueryState((prevState) => ({
        ...prevState,
        anchorStatus: { value: LOADING_STATUS.FAILED, reason: FAILURE_REASONS.UNKNOWN },
      }));
      toastNotifications.addDanger({
        title: i18n.translate('discover.context.unableToLoadAnchorDocumentDescription', {
          defaultMessage: 'Unable to fetch anchor document',
        }),
        text: 'fail',
      });
    }
  }, [anchorId, indexPattern, searchSource, tieBreakerField, toastNotifications]);

  const fetchSurroundingRows = useCallback(
    async (type: SurrDocType, count: number, filters: Filter[], anchor?: OpenSearchHitRecord) => {
      try {
        if (type === SurrDocType.PREDECESSORS) {
          setContextQueryState((prevState) => ({
            ...prevState,
            predecessorsStatus: { value: LOADING_STATUS.LOADING },
          }));
        } else {
          setContextQueryState((prevState) => ({
            ...prevState,
            successorsStatus: { value: LOADING_STATUS.LOADING },
          }));
        }
        const fetchedAchor = anchor || contextQueryState.anchor;

        const rows = await fetchSurroundingDocs(
          type,
          indexPattern,
          fetchedAchor as OpenSearchHitRecord,
          tieBreakerField,
          SortDirection.desc,
          count,
          filters
        );
        if (type === SurrDocType.PREDECESSORS) {
          setContextQueryState((prevState) => ({
            ...prevState,
            predecessors: rows,
            predecessorsStatus: { value: LOADING_STATUS.LOADED },
          }));
        } else {
          setContextQueryState((prevState) => ({
            ...prevState,
            successors: rows,
            successorsStatus: { value: LOADING_STATUS.LOADED },
          }));
        }
      } catch (error) {
        if (type === SurrDocType.PREDECESSORS) {
          setContextQueryState((prevState) => ({
            ...prevState,
            predecessorsStatus: { value: LOADING_STATUS.FAILED, reason: FAILURE_REASONS.UNKNOWN },
          }));
        } else {
          setContextQueryState((prevState) => ({
            ...prevState,
            successorsStatus: { value: LOADING_STATUS.FAILED, reason: FAILURE_REASONS.UNKNOWN },
          }));
        }
        toastNotifications.addDanger({
          title: i18n.translate('discover.context.unableToLoadSurroundingDocumentDescription', {
            defaultMessage: 'Unable to fetch surrounding documents',
          }),
          text: 'fail',
        });
      }
    },
    [contextQueryState.anchor, indexPattern, tieBreakerField, toastNotifications]
  );

  const fetchContextRows = useCallback(
    async (
      predecessorCount: number,
      successorCount: number,
      filters: Filter[],
      anchor?: OpenSearchHitRecord
    ) =>
      Promise.all([
        fetchSurroundingRows(SurrDocType.PREDECESSORS, predecessorCount, filters, anchor),
        fetchSurroundingRows(SurrDocType.SUCCESSORS, successorCount, filters, anchor),
      ]),
    [fetchSurroundingRows]
  );

  const fetchAllRows = useCallback(
    async (predecessorCount: number, successorCount: number, filters: Filter[]) => {
      try {
        await fetchAnchorRow().then(
          (anchor) => anchor && fetchContextRows(predecessorCount, successorCount, filters, anchor)
        );
      } catch (error) {
        toastNotifications.addDanger({
          title: i18n.translate('discover.context.unableToLoadDocumentDescription', {
            defaultMessage: 'Unable to fetch all documents',
          }),
          text: 'fail',
        });
      }
    },
    [fetchAnchorRow, fetchContextRows, toastNotifications]
  );

  const resetContextQueryState = useCallback(() => {
    setContextQueryState(initialState);
  }, []);

  return {
    contextQueryState,
    fetchAnchorRow,
    fetchAllRows,
    fetchContextRows,
    fetchSurroundingRows,
    resetContextQueryState,
  };
}

The fetchAnchorRow call will fail immediately because the tieBreakerField validation occurs at the start:

const fetchAnchorRow = useCallback(async () => {
    if (!tieBreakerField) {
      setContextQueryState((prevState) => ({
        ...prevState,
        anchorStatus: {
          value: LOADING_STATUS.FAILED,
          reason: FAILURE_REASONS.INVALID_TIEBREAKER,
        },
      }));
      return;
    }
    // ... rest of the function

If no valid tieBreakerField, for example currency for sample flights, these will happen:

const tieBreakerField = useMemo(
  () => getFirstSortableField(indexPattern, uiSettings.get(CONTEXT_TIE_BREAKER_FIELDS_SETTING)),
  [uiSettings, indexPattern]
 );

export function getFirstSortableField(indexPattern: IndexPattern, fieldNames: string[]) {
 const sortableFields = fieldNames.filter(
  (fieldName) =>
   META_FIELD_NAMES.includes(fieldName) ||
   // @ts-ignore
   (indexPattern.fields.getByName(fieldName) || { sortable: false }).sortable
 );
 return sortableFields[0];
}
  • tieBreakerField is set to undefined
  • set the anchorStatus to LOADING_STATUS.FAILED
  • set the failure reason to FAILURE_REASONS.INVALID_TIEBREAKER
  • return early before making any API calls
  • Since fetchAllRows depends on fetchAnchorRow, any calls to fetchAllRows will also fail.

To Reproduce
Steps to reproduce the behavior:

  1. Go to 'Advanced settings'
  2. Set tieBreakerField to [currency]
  3. Go back to discover and choose sample flights
  4. See error

Expected behavior
a fall back and not break usage

OpenSearch Version
all

Dashboards Version
all

@ananzh ananzh added bug Something isn't working discover for discover reinvent labels Oct 29, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working discover for discover reinvent
Projects
None yet
Development

No branches or pull requests

1 participant