[
https://issues.apache.org/jira/browse/TINKERPOP-2708?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=17551802#comment-17551802
]
ASF GitHub Bot commented on TINKERPOP-2708:
-------------------------------------------
codecov-commenter commented on PR #1680:
URL: https://github.com/apache/tinkerpop/pull/1680#issuecomment-1150439765
#
[Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1680?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
Report
> Merging
[#1680](https://codecov.io/gh/apache/tinkerpop/pull/1680?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
(4fc9e8d) into
[3.5-dev](https://codecov.io/gh/apache/tinkerpop/commit/945e588baf7f04e3f95e1f2756b4558bb27df732?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
(945e588) will **not change** coverage.
> The diff coverage is `n/a`.
> :exclamation: Current head 4fc9e8d differs from pull request most recent
head 199182a. Consider uploading reports for the commit 199182a to get more
accurate results
```diff
@@ Coverage Diff @@
## 3.5-dev #1680 +/- ##
========================================
Coverage 63.35% 63.35%
========================================
Files 23 23
Lines 3553 3553
========================================
Hits 2251 2251
Misses 1129 1129
Partials 173 173
```
------
[Continue to review full report at
Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1680?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn
more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by
[Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1680?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
Last update
[945e588...199182a](https://codecov.io/gh/apache/tinkerpop/pull/1680?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
Read the [comment
docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> 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
> Priority: Major
>
> 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.7#820007)
