Fix error message display issue in dev tool console

[zhongnansu]

Description

• Fix error message display issue in dev tool console

Updated screenshot

Previous screenshot

Issues Resolved

#3645

Check List

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

[codecov-commenter]

Codecov Report

Merging #3652 (b2d90c8) into main (de06344) will decrease coverage by 0.05%.
The diff coverage is 40.00%.

📣 This organization is not using Codecov’s GitHub App Integration. We recommend you install it so Codecov can continue to function properly for your repositories. Learn more

@@            Coverage Diff             @@
##             main    #3652      +/-   ##
==========================================
- Coverage   66.45%   66.41%   -0.05%     
==========================================
  Files        3208     3209       +1     
  Lines       61593    61610      +17     
  Branches     9502     9504       +2     
==========================================
- Hits        40932    40916      -16     
- Misses      18384    18413      +29     
- Partials     2277     2281       +4     

Flag	Coverage Δ
Linux	?
Windows	66.41% <40.00%> (+<0.01%)	⬆️

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

Impacted Files	Coverage Δ
.../server/routes/api/console/proxy/create_handler.ts	74.35% <25.00%> (-2.57%)	⬇️
src/core/server/opensearch/client/errors.ts	100.00% <100.00%> (ø)

... and 9 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

[kavilla]

Hey @zhongnansu,

Thanks for handling this.

However, it still not quite formatted for readability and it dumps the error to the log whereas prior it did not. Would it possible to get it to feature parity ?

[zhongnansu]

@kavilla valid point, found a way to return exact error message as existing dev tool. Please take a look

[seraphjiang]

is this issue introduced with new dev tool change for MD? if so, assume previous version doesn't has such issue, do we still need to callout and add to changelog?

[zhongnansu]

It's introduced by the dev tool refactor PR, literally it's not even related to MD. It's a good point, previous version doesn't have issue. Currently it's only that each PR is mapped to at least one changelog.

Any thoughts? @kavilla @abbyhu2000

[joshuarrrr]

yep, @seraphjiang is correct - if we're only fixing an unreleased bug, no need for a new changelog entry. In some cases we would maybe want to update the original changelog entry, but in this case, I'd just skip.

And when we generate the release notes, we'll want to do some additional cleanup and consolidation, so that all the MD entries are grouped and nicely formatted.

[zhongnansu]

removed change log item

[kavilla]

Hey @zhongnansu, looks a lot more human readable!

Are we able to resolve the issue with it outputting error logs?

For example, on 2.6 if you hit an exception you can check your OpenSearch Dashboards server logs and it does not output anything.

But for this I am seeing:

[error][console][plugins] ResponseError: index_not_found_exception: [index_not_found_exception] Reason: no such index [foobar]
    at onBody (/path/node_modules/@opensearch-project/opensearch/lib/Transport.js:403:23)
    at IncomingMessage.onEnd (/pathnode_modules/@opensearch-project/opensearch/lib/Transport.js:318:11)
    at IncomingMessage.emit (events.js:412:35)
    at IncomingMessage.emit (domain.js:475:12)
    at endReadableNT (internal/streams/readable.js:1333:12)
    at processTicksAndRejections (internal/process/task_queues.js:82:21) {
  meta: {
    body: { error: [Object], status: 404 },
    statusCode: 404,
    headers: {
      'x-opaque-id': '####',
      'content-type': 'application/json; charset=UTF-8',
      'content-length': '514'
    },
    meta: {
      context: null,
      request: [Object],
      name: 'opensearch-js',
      connection: [Object],
      attempts: 0,
      aborted: false
    }
  }
}

[zhongnansu]

@kavilla Make sense, since client side will get the error for the user query , no need to print at console server side.I removed the log line

[abbyhu2000]

@zhongnansu On 2.6, the console does not output anything, but when I ran this, i still get some error log output:

server log [19:41:05.242] [error][data][opensearch] [index_not_found_exception]: no such index [my-dql-sample2]

[zhongnansu]

@zhongnansu On 2.6, the console does not output anything, but when I ran this, i still get some error log output:

server log [19:41:05.242] [error][data][opensearch] [index_not_found_exception]: no such index [my-dql-sample2]

That's because after the refactor, the query request is now going through client provided by routeHander in core. Similar as user render visualization/Discover, it will log the error in core and label it as [data][opensearch]. Fact is user did make a request to OpenSearch and failed, I don't see a issue with core side logging the error.

I don't think it's a blocking issue. But if you have concern around UX, you can open another issue for discuss, maybe also in UX repo? I suggest we move forward with this PR. Thanks for bring it up. @abbyhu2000

[kristenTian]

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.

[zhongnansu]

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