You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@tinkerpop.apache.org by "iAmmarTahir (via GitHub)" <gi...@apache.org> on 2023/01/30 11:28:08 UTC
[GitHub] [tinkerpop] iAmmarTahir opened a new pull request, #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
iAmmarTahir opened a new pull request, #1952:
URL: https://github.com/apache/tinkerpop/pull/1952
While using Gremlin-javascript in production, I faced an undefined error in `lib/utils.js`. Further investigation revealed that the fields were not correctly checked for nulls. This PR aims to add proper null checks to avoid undefined errors on runtime.
The error screenshot:
<img width="446" alt="Screenshot 2023-01-30 at 4 24 22 PM" src="https://user-images.githubusercontent.com/41549829/215464175-79faf65f-7917-4f90-baf5-bb4270fd9caf.png">
Gremlin version: v3.6.2
Node version: v14.17.4
NPM version: 7.19.1
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #1952:
URL: https://github.com/apache/tinkerpop/pull/1952#issuecomment-1414483480
I also want to note that this PR failed the JS CI tests as the linter is currently configured for ecma 2017 which is roughly equivalent to node v10. I have submitted a [PR](https://github.com/apache/tinkerpop/pull/1959) to bump this to version 2021 which is fully supported by node v16 and will prevent the linter from rejecting the use of `?.`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] iAmmarTahir commented on pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
Posted by "iAmmarTahir (via GitHub)" <gi...@apache.org>.
iAmmarTahir commented on PR #1952:
URL: https://github.com/apache/tinkerpop/pull/1952#issuecomment-1414989929
@Cole-Greer I have addressed the issues that you mentioned and removed optional chaining to support compatibility for NodeJS v10. I will target this branch towards `3.5-dev`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] Cole-Greer commented on pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
Posted by "Cole-Greer (via GitHub)" <gi...@apache.org>.
Cole-Greer commented on PR #1952:
URL: https://github.com/apache/tinkerpop/pull/1952#issuecomment-1414457774
@iAmmarTahir Thanks for catching this error and submitting this fix. I would like to add that this would be further improved if a default value of 'NotAvailable' was provided as in ``` const cpuArch = process?.arch?.replace(' ', '_') || 'NotAvailable';```
Also I would like to ask if you could target this PR to 3.5-dev or 3.6-dev. Our master branch contains the code which will eventually make up the next major release (3.7.0). To have your changes brought into upcoming minor releases (3.5.6, 3.6.3) the changes will need to be in the corresponding dev branches. You should always target the earliest branch you wish to support as changes will be merged forward (eg. if your PR is merged into 3.5-dev, we will also merge it forward into 3.6-dev and master).
One wrinkle is that the 3.5.x branch is using node v10 which does not support the `?.` operator. Therefore some less elegant null checks will be needed for that branch. If you are willing to submit a node v10 compatible fix to 3.5-dev that would be fantastic. Otherwise I would ask that you add the default value of 'NotAvailable' and target this PR to 3.6-dev and I can look into a fix for the 3.5.x branch.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] codecov-commenter commented on pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #1952:
URL: https://github.com/apache/tinkerpop/pull/1952#issuecomment-1408479290
# [Codecov](https://codecov.io/gh/apache/tinkerpop/pull/1952?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 [#1952](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (778176c) into [master](https://codecov.io/gh/apache/tinkerpop/commit/fe8e36cdd0829767886c3dffa6ebcf98dd96cfd3?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (fe8e36c) will **decrease** coverage by `4.28%`.
> The diff coverage is `n/a`.
```diff
@@ Coverage Diff @@
## master #1952 +/- ##
============================================
- Coverage 68.54% 64.27% -4.28%
============================================
Files 854 24 -830
Lines 41185 3728 -37457
Branches 5598 0 -5598
============================================
- Hits 28231 2396 -25835
+ Misses 10980 1166 -9814
+ Partials 1974 166 -1808
```
| [Impacted Files](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...op/gremlin/structure/io/graphml/GraphMLWriter.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vZ3JhcGhtbC9HcmFwaE1MV3JpdGVyLmphdmE=) | | |
| [...n/process/traversal/step/sideEffect/StartStep.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdGVwL3NpZGVFZmZlY3QvU3RhcnRTdGVwLmphdmE=) | | |
| [...ucture/io/graphson/ToStringGraphSONSerializer.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vZ3JhcGhzb24vVG9TdHJpbmdHcmFwaFNPTlNlcmlhbGl6ZXIuamF2YQ==) | | |
| [.../apache/tinkerpop/gremlin/console/PluggedIn.groovy](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb25zb2xlL3NyYy9tYWluL2dyb292eS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL2NvbnNvbGUvUGx1Z2dlZEluLmdyb292eQ==) | | |
| [...inkerpop/gremlin/structure/io/gryo/GryoWriter.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vZ3J5by9HcnlvV3JpdGVyLmphdmE=) | | |
| [...3/dsl/credential/CredentialGraphGremlinPlugin.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1ncm9vdnkvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL2dyb292eS9qc3IyMjMvZHNsL2NyZWRlbnRpYWwvQ3JlZGVudGlhbEdyYXBoR3JlbWxpblBsdWdpbi5qYXZh) | | |
| [...al/strategy/optimization/RepeatUnrollStrategy.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9wcm9jZXNzL3RyYXZlcnNhbC9zdHJhdGVneS9vcHRpbWl6YXRpb24vUmVwZWF0VW5yb2xsU3RyYXRlZ3kuamF2YQ==) | | |
| [...emlin/structure/io/binary/types/SetSerializer.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9zdHJ1Y3R1cmUvaW8vYmluYXJ5L3R5cGVzL1NldFNlcmlhbGl6ZXIuamF2YQ==) | | |
| [...gremlin/server/util/DefaultTemporaryException.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1zZXJ2ZXIvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3RpbmtlcnBvcC9ncmVtbGluL3NlcnZlci91dGlsL0RlZmF1bHRUZW1wb3JhcnlFeGNlcHRpb24uamF2YQ==) | | |
| [...emlin/language/grammar/GremlinParserException.java](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-Z3JlbWxpbi1jb3JlL3NyYy9tYWluL2phdmEvb3JnL2FwYWNoZS90aW5rZXJwb3AvZ3JlbWxpbi9sYW5ndWFnZS9ncmFtbWFyL0dyZW1saW5QYXJzZXJFeGNlcHRpb24uamF2YQ==) | | |
| ... and [820 more](https://codecov.io/gh/apache/tinkerpop/pull/1952?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [tinkerpop] iAmmarTahir closed pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
Posted by "iAmmarTahir (via GitHub)" <gi...@apache.org>.
iAmmarTahir closed pull request #1952: patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors
URL: https://github.com/apache/tinkerpop/pull/1952
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: commits-unsubscribe@tinkerpop.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org