hazelcast / hazelcast-nodejs-client

Hazelcast Node.js Client
https://hazelcast.com/clients/node-js/
Apache License 2.0
150 stars 59 forks source link

Change logic around closing connections and writers API-1283 #1417

Closed srknzl closed 1 year ago

srknzl commented 1 year ago

fixes #1256

The issue was writing some data to the connection after it being closed. I changed and refactored some logic around PipelinedWriter and Connection to avoid that.

codecov[bot] commented 1 year ago

Codecov Report

Merging #1417 (96d534f) into master (2ac53b4) will decrease coverage by 0.07%. The diff coverage is 84.61%.

@@            Coverage Diff             @@
##           master    #1417      +/-   ##
==========================================
- Coverage   93.27%   93.20%   -0.08%     
==========================================
  Files         464      466       +2     
  Lines       16386    16623     +237     
  Branches     1333     1351      +18     
==========================================
+ Hits        15284    15493     +209     
- Misses        801      827      +26     
- Partials      301      303       +2     
Impacted Files Coverage Δ
src/network/Connection.ts 94.82% <84.61%> (+0.60%) :arrow_up:
src/network/ConnectionManager.ts 79.81% <0.00%> (-5.17%) :arrow_down:
src/core/UUID.ts 85.00% <0.00%> (-5.00%) :arrow_down:
src/codec/SqlExecuteCodec.ts 95.74% <0.00%> (-4.26%) :arrow_down:
src/util/Util.ts 86.30% <0.00%> (-1.60%) :arrow_down:
src/protocol/ErrorFactory.ts 64.66% <0.00%> (-0.76%) :arrow_down:
src/core/HazelcastError.ts 77.61% <0.00%> (-0.42%) :arrow_down:
src/serialization/ObjectData.ts 88.93% <0.00%> (-0.40%) :arrow_down:
src/config/ConfigBuilder.ts 90.29% <0.00%> (-0.39%) :arrow_down:
src/core/index.ts 100.00% <0.00%> (ø)
... and 121 more

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

srknzl commented 1 year ago

Note regarding coverage: The coverage drop is shown as 0.16% but I see that the drop is due to SSL tests because the drop is in connectTLSSocket. I think it should be related to the ssl tests being disabled. (Due to certificate issue)

Also note that codecov does not show a line that is added but not covered in this PR. (except a single line, I explained it above)