[x] Added tests for code changes or test/build only changes
[x] Updated the change log file (CHANGES.md|CHANGELOG.md) or test/build only changes
[x] Completed the PR template below:
Description
Correct opportunities for double callbacks
Fixes #454
Approach
When writing tests I noticed that max-age was not correctly parsed from a single set cookie header (only from an array) - so there is one fix to resolve that issue.
Double callback (case 1):
In auth plugins for case where an error is encountered during state update.
The cookieauth and iamauth plugins are modified to callback(state) in a finally at the end of a promise chain instead of from two places in the onRequest function.
Double callback (case 2):
Discovered when updating nock to 13.x (see Test section below). In the case a request is aborted for a retry a double request end event might be received because (to more precisely mirror current Node behaviours the abort event moved into the next tick. To ensure our plugins behave correctly with the aborts the callbacks in lib/clientutils.jsprocessState are deferred to the next tick.
Schema & API Changes
Remove EOL Node.js versions from test matrix and supported engines
Security and Privacy
Fix package-lock.json for a npm audit warning
Testing
Moved test/mocha.opts -> .mocharc.json to avoid warning for using deprecated format.
Existing tests provide good coverage that functionality is unaffected.
For the case 1 double callback I could monkey-patch the state update to throw an error to demonstrate the problem (and did so during manual testing), but I don't think it would provide any value in automated test.
As I was initially investigating I wrote a couple of extra tests for some IAM auth error cases.
Upgraded nock to 13.x (to resolve deprecation warnings from Node versions >=10)
Caused CloudantClient/#db using listeners/with ComplexPlugin2 to fail with the end event being triggered twice, causing two done callbacks. This is similar to the reported issue so another fix (described above) was introduced to resolve the failure.
Checklist
CHANGES.md
|CHANGELOG.md
) or test/build only changesDescription
Correct opportunities for double callbacks
Fixes #454
Approach
When writing tests I noticed that max-age was not correctly parsed from a single set cookie header (only from an array) - so there is one fix to resolve that issue.
Double callback (case 1): In auth plugins for case where an error is encountered during state update. The
cookieauth
andiamauth
plugins are modified tocallback(state)
in afinally
at the end of a promise chain instead of from two places in theonRequest
function.Double callback (case 2): Discovered when updating nock to 13.x (see Test section below). In the case a request is aborted for a retry a double request
end
event might be received because (to more precisely mirror current Node behaviours theabort
event moved into the next tick. To ensure our plugins behave correctly with the aborts the callbacks inlib/clientutils.js
processState
are deferred to the next tick.Schema & API Changes
Security and Privacy
Testing
test/mocha.opts
->.mocharc.json
to avoid warning for using deprecated format.CloudantClient/#db using listeners/with ComplexPlugin2
to fail with theend
event being triggered twice, causing two done callbacks. This is similar to the reported issue so another fix (described above) was introduced to resolve the failure.Monitoring and Logging