[
https://issues.apache.org/jira/browse/TINKERPOP-2708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=18071538#comment-18071538
]
ASF GitHub Bot commented on TINKERPOP-2708:
-------------------------------------------
codecov-commenter commented on PR #1701:
URL: https://github.com/apache/tinkerpop/pull/1701#issuecomment-4195039380
##
[Codecov](https://app.codecov.io/gh/apache/tinkerpop/pull/1701?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)
Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 63.84%. Comparing base
([`7dd393d`](https://app.codecov.io/gh/apache/tinkerpop/commit/7dd393d8420264e6fc8743d9dd4669b53e07567c?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache))
to head
([`d3cf21a`](https://app.codecov.io/gh/apache/tinkerpop/commit/d3cf21a741cf016605c99e61df47eda75febf2f7?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
:warning: Report is 3397 commits behind head on master.
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## master #1701 +/- ##
=======================================
Coverage 63.84% 63.84%
=======================================
Files 23 23
Lines 3596 3596
=======================================
Hits 2296 2296
Misses 1137 1137
Partials 163 163
```
</details>
[:umbrella: View full report in Codecov by
Sentry](https://app.codecov.io/gh/apache/tinkerpop/pull/1701?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it
here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test
Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests,
report on failures, and find test suite problems.
- :package: [JS Bundle
Analysis](https://docs.codecov.com/docs/javascript-bundle-analysis): Save
yourself from yourself by tracking and limiting bundle sizes in JS merges.
</details>
> unhandledRejection upon connection failure
> ------------------------------------------
>
> Key: TINKERPOP-2708
> URL: https://issues.apache.org/jira/browse/TINKERPOP-2708
> Project: TinkerPop
> Issue Type: Bug
> Components: javascript
> Affects Versions: 3.5.2
> Reporter: Jon Brede Skaug
> Assignee: Stephen Mallette
> Priority: Major
> Labels: breaking
> Fix For: 3.7.0
>
>
> In the Javascript driver is unable to connect to the graph database for
> whatever reason an unhandledRejection warning occurs.
> I have tested this with `new DriverRemoteConnection`
> This is a silent error and it won't be able to catch the error due to the way
> it is handled.
> I've tracked it down to this line:
> [https://github.com/apache/tinkerpop/blob/c22c0141bb7a00f366f929d0e5d3c6379d1004e0/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js#L156]
> h2. *Solution suggestion*
> A fairly quick solution (but possibly breaking change) to this is by not
> opening the database in the constructor [(line reference
> L105)|https://github.com/apache/tinkerpop/blob/c22c0141bb7a00f366f929d0e5d3c6379d1004e0/gremlin-javascript/src/main/javascript/gremlin-javascript/lib/driver/connection.js#L105]
> but instead forcing the user to run the `DriverRemoteConnection.open()`
> after the constructor has been initialized. `DriverRemoteConnection.open()`
> returns a promise which makes more sense and is a bit more intuitive. The
> current error message gives an error about DNS which is increadibly confusing
> without deepdiving into the Gremlin driver code and navigating through 3
> classes to find the culprit. It's also an error which seems a bit more
> harmless than it actually is.
> It'salso possible to set option.connectOnStartup to "false" by default, this
> however will require the user to be aware of the possible failure upon
> setting it to true. I believe forcing the user to run .open() after
> initializing the class may be more robust.
> By doing it this way the user can instead handle the error raised by
> DriverRemoteConnection.open() by using promise.catch() or an async function
> using await. Promise.catch() is as provided:
> {code:java}
> this.drc.open().catch(err => {
> console.log("Unable to open connection to database", err);
> });{code}
> h2. *{{Temporary work around example}}*
>
> {code:java}
> // Using promises
> const drc = new DriverRemoteConnection(url, {connectOnStartup: false});
> drc.open().catch(err => {
> // Handle error upon open, i.e using retry and backoff logic, notify an
> alarm system or setting a global variable to reject requests.
> });{code}
>
> h2. *The issue with not handling the error properly:*
> Not handling the error properly means that if you pass in an invalid URL or
> the gremlin compatible database is down, it won't be able to handle the
> connection error before a transaction is attempted.
> In the future Node.js unhandledRejection will terminate the Node.js process.
> This can cause critical failure of processes upon boot and may even cause
> DDoS situations where processes may flood the gremlin compatible database
> with connection attempts due to processes failing and being reinstated over
> and over by a process monitor.
>
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
