fix(discover): disable cache only on indexPatterns.get to prevent crash

[joshuali925]

Description

Right now in new discover, if user adds/removes two fields on the left quickly, it will crash with

Something went wrong
Try refreshing the page. If that doesn't work, go back to the previous page or clear your session data.


Clear your session

Go back
Cannot read properties of undefined (reading 'type')
Version: 3.0.0
Build: 9007199254740991
TypeError: Cannot read properties of undefined (reading 'type')

console error

fatal_errors_service.tsx:89 TypeError: Cannot read properties of undefined (reading 'type')
    at isDefault (is_default.ts:33:1)
    at updateSearchSource (update_search_source.ts:32:1)
    at async use_search.ts:101:1
    at async use_search.ts:193:1

I believe this happens because cache is being updated when user adds a field, and the index pattern is not available in the cache during the update.

OpenSearch-Dashboards/src/plugins/discover/public/application/helpers/popularize_field.ts

Lines 45 to 49 in a9f84be

	// Catch 409 errors caused by user adding columns in a higher frequency that the changes can be persisted to OpenSearch
	try {
	await indexPatternsService.updateSavedObject(indexPattern, 0, true);
	// eslint-disable-next-line no-empty
	} catch {}

If user adds another field when cache is not available, the index pattern will be undefined, and calling undefined.type crashes the page

OpenSearch-Dashboards/src/plugins/data/common/index_patterns/lib/is_default.ts

Lines 33 to 36 in a9f84be

	export const isDefault = (indexPattern: IIndexPattern) => {
	// Default index patterns don't have `type` defined.
	return !indexPattern.type;
	};

Issues Resolved

Screenshot

Testing the changes

Changelog

• skip

Check List

• All tests pass
  • yarn test:jest
  • yarn test:jest_integration
• New functionality includes testing.
• New functionality has been documented.
• Update CHANGELOG.md
• Commits are signed per the DCO using --signoff

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 64.29%. Comparing base (1f4cbbb) to head (a9f84be).
Report is 21 commits behind head on main.

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #7926   +/-   ##
=======================================
  Coverage   64.29%   64.29%           
=======================================
  Files        3674     3674           
  Lines       81149    81149           
  Branches    12928    12928           
=======================================
+ Hits        52173    52175    +2     
+ Misses      25765    25764    -1     
+ Partials     3211     3210    -1     

Flag	Coverage Δ
Linux_1	30.20% <ø> (ø)
Linux_2	56.19% <ø> (ø)
Linux_3	40.61% <ø> (+<0.01%)	⬆️
Linux_4	31.40% <ø> (ø)
Windows_1	30.22% <ø> (ø)
Windows_2	56.14% <ø> (ø)
Windows_3	40.61% <ø> (ø)
Windows_4	31.40% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[kavilla]

do we know if this works fine with indices on load?

[joshuali925]

do we know if this works fine with indices on load?

the fields from the last index-pattern search are showed on the left when searching indices, seems to be another bug. for testing i searched the index with the same fields, and adding/removing fields worked

[AMoo-Miki]

Spoke to @joshuali925; the true removal makes it so the dataset doesn't rely on cache.

[joshuali925]

adding some context, i sent this PR because page crash affects user experience and i want to fix it for 2.17. this seems to be the easiest fix, i'm not sure what is the best fix

[AMoo-Miki]

Spoke to Rocky offline:

joshua's work will address this issue. it's a good observation regarding MQL. in the initial implementation, before we added the data explorer component in version 2.17, i created a temporary index pattern for the results. this was necessary because the search source required an explicit index pattern.
previously, the process worked like:
check if the index pattern exists in the cache
if not found in the cache, query the saved objects client
if still not found, attempt to query the cluster for the index
For PPL and SQL queries, I would:
create a dataframe
if the dataframe existed on the search source, create a temporary index pattern
insert this temporary pattern only in the cache (not creating a saved object)
bypass the normal logic and only check the index pattern cache if the dataframe existed in the search source
which prevented potential errors that could occur when querying an index or table. with the explorer component, we create the temporary index pattern much earlier in the process, before any search source interactions. which allows us to rely on the normal code path to work properly, eliminating the need for the previous workaround