[2.x-manual-bp][Discover] Fix error handling for ppl jobs API (#8771)

[ananzh]

Description

Backport PR:
#8771

From 8771:

• fix error handling for ppl jobs API

Changelog

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

[github-actions]

❌ Empty Changelog Section

The Changelog section in your PR description is empty. Please add a valid changelog entry or entries. If you did add a changelog entry, check to make sure that it was not accidentally included inside the comment block in the Changelog section.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 51.73%. Comparing base (1cc5c0f) to head (8d847a5).
Report is 1 commits behind head on 2.x.

❗ There is a different number of reports uploaded between BASE (1cc5c0f) and HEAD (8d847a5). Click for more details.

HEAD has 2 uploads less than BASE

Flag	BASE (1cc5c0f)	HEAD (8d847a5)
Linux_4	1	0
Windows_2	1	0

Additional details and impacted files

@@            Coverage Diff             @@
##              2.x    #8777      +/-   ##
==========================================
- Coverage   60.81%   51.73%   -9.09%     
==========================================
  Files        3797     3513     -284     
  Lines       90711    86798    -3913     
  Branches    14274    13655     -619     
==========================================
- Hits        55169    44905   -10264     
- Misses      32091    38812    +6721     
+ Partials     3451     3081     -370     

Flag	Coverage Δ
Linux_1	29.06% <ø> (ø)
Linux_4	?
Windows_1	29.07% <ø> (ø)
Windows_2	?
Windows_3	37.64% <ø> (ø)
Windows_4	29.94% <ø> (ø)

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.

[AMoo-Miki]

queryStatusResponse could be undefined.

[ananzh]

So this describeQuery calls fetchJobs or fetch. Both wraps the API call in a try-catch block: If the API call succeeds, it returns { success: true, data: queryRes }. If the API call fails, it returns { success: false, data: err }. So the result is either return { ...response, data: shimFunctionsdataType } or response, with success either true or false right?

So the result from describeQuery could be

 if (response.success === false || !this.shimResponse) return response;

where it has success either true or false.

or

shimFunctions[dataType]
      ? { ...response, data: shimFunctions[dataType](response.data) }
      : response;

where it still has response in both ways. So seems the exception for describeQuery can be captured in facet error right? What do you think?

[joshuali925]

it won't be undefined:

OpenSearch-Dashboards/src/plugins/query_enhancements/server/utils/facet.ts

Line 99 in 8d847a5

public describeQuery = async (context: any, request: any): Promise<FacetResponse> => {

OpenSearch-Dashboards/src/plugins/query_enhancements/server/types.ts

Lines 48 to 51 in 8d847a5

	export interface FacetResponse {
	success: boolean;
	data: any;
	}

[AMoo-Miki]

The issue i am having is queryStatusResponse: any. Will need to understand why the code doesn't use queryStatusResponse: FacetResponse and why the nest line does queryStatusResponse?.data.

[joshuali925]

yes i don't know why it uses any, type can be omitted since for describeQuery, the return type is correct and there are no type errors in it

data shouldn't be any as well, it should be either Error or the response type.

And i think the real problem is that i'm not sure why it goes from

error response -> catch -> return with `success: false` -> check if `success === false` -> throw new error wrapping response error

instead of just

error response -> throw response error

in the current logic it's likely to miss checking success === false. This PR and my PR #8743 were fixing the same problem that this check is missing

Dismissing to unblock release but a followup is warranted.