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.

Sign up for GitHub

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

Fix error message display issue in dev tool console #3652

Merged
merged 5 commits into from
Mar 27, 2023
Merged

Fix error message display issue in dev tool console #3652

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
0223104
Fix error message display issue in dev tool console
zhongnansu Mar 22, 2023
040b333
update logic
zhongnansu Mar 23, 2023
82a6c98
minor update
zhongnansu Mar 23, 2023
96b90ef
remove changelog item
zhongnansu Mar 23, 2023
b2d90c8
remove logging
zhongnansu Mar 24, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/core/server/opensearch/client/errors.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,7 @@ export type UnauthorizedError = ResponseError & {
};

export function isResponseError(error: any): error is ResponseError {
return Boolean(error.body && error.statusCode && error.headers);
return Boolean(error && error.body && error.statusCode && error.headers);
}

export function isUnauthorizedError(error: any): error is UnauthorizedError {
Expand Down
23 changes: 16 additions & 7 deletions src/plugins/console/server/routes/api/console/proxy/create_handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,14 @@

import { OpenSearchDashboardsRequest, RequestHandler } from 'opensearch-dashboards/server';
import { trimStart } from 'lodash';
import { Readable } from 'stream';

import { ResponseError } from '@opensearch-project/opensearch/lib/errors';
import { ApiResponse } from '@opensearch-project/opensearch/';

// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { ensureRawRequest } from '../../../../../../../core/server/http/router';
// eslint-disable-next-line @osd/eslint/no-restricted-paths
import { isResponseError } from '../../../../../../../core/server/opensearch/client/errors';

import { RouteDependencies } from '../../../';

Expand Down Expand Up @@ -127,15 +129,22 @@ export const createHandler = ({
},
});
} catch (e: any) {
log.error(e);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason why we don't want to log the error? It's good practice to have service logging the errors for analysis or troubleshooting.

Copy link
Member Author

@zhongnansu zhongnansu Mar 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We are logging error by core->openesarch logger, but not by console logger. Because the error response will be forwarded to client(browser). No need to log 2 times

const isResponseErrorFlag = isResponseError(e);

const errorMessage = isResponseErrorFlag ? JSON.stringify(e.meta.body) : e.message;
// core http route handler has special logic that asks for stream readable input to pass error opaquely
const errorResponseBody = new Readable({
read() {
this.push(errorMessage);
this.push(null);
kristenTian marked this conversation as resolved.
Show resolved Hide resolved
},
});
return response.customError({
statusCode: isResponseErrorFlag ? e.statusCode : 502,
body: isResponseErrorFlag ? JSON.stringify(e.meta.body) : `502.${e.statusCode || 0}`,
body: errorResponseBody,
headers: {
'Content-Type': 'application/json',
},
});
}
};

const isResponseError = (error: any): error is ResponseError => {
return Boolean(error && error.body && error.statusCode && error.header);
};