patch: Adding for null checks in generateUserAgent() to avoid undefined runtime errors

[iAmmarTahir]

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:

Gremlin version: v3.6.2
Node version: v14.17.4
NPM version: 7.19.1

[codecov-commenter]

Codecov Report

Merging #1952 (778176c) into master (fe8e36c) will increase coverage by 0.06%.
The diff coverage is n/a.

@@             Coverage Diff              @@
##             master    #1952      +/-   ##
============================================
+ Coverage     68.54%   68.61%   +0.06%     
- Complexity     9088     9098      +10     
============================================
  Files           854      854              
  Lines         41185    41185              
  Branches       5598     5598              
============================================
+ Hits          28231    28259      +28     
+ Misses        10980    10951      -29     
- Partials       1974     1975       +1     

Impacted Files	Coverage Δ
...kergraph/process/computer/TinkerGraphComputer.java	95.41% <0.00%> (-0.77%)	⬇️
...in/process/traversal/dsl/graph/GraphTraversal.java	90.70% <0.00%> (ø)
...pache/tinkerpop/gremlin/driver/ConnectionPool.java	29.54% <0.00%> (+0.64%)	⬆️
...mlin/server/op/traversal/TraversalOpProcessor.java	48.38% <0.00%> (+0.92%)	⬆️
...a/org/apache/tinkerpop/gremlin/server/Context.java	83.09% <0.00%> (+1.40%)	⬆️
...java/org/apache/tinkerpop/gremlin/driver/Host.java	37.77% <0.00%> (+2.22%)	⬆️
.../step/map/ConnectedComponentVertexProgramStep.java	76.47% <0.00%> (+5.88%)	⬆️
...rg/apache/tinkerpop/gremlin/driver/Connection.java	64.37% <0.00%> (+10.62%)	⬆️
.../gremlin/driver/exception/ConnectionException.java	44.44% <0.00%> (+22.22%)	⬆️

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

[Cole-Greer]

@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.

[Cole-Greer]

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 to bump this to version 2021 which is fully supported by node v16 and will prevent the linter from rejecting the use of ?.

[iAmmarTahir]

@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