cockatiel - npm Package Compare versions

Comparing version 1.1.0 to 1.1.1

changelog.md

 # Changelog
 
+## 1.1.1 - 2020-07-17
+
+- **fix:** events on the timeout policy being emitted incorrectly, or not emitted (see [#27](https://github.com/connor4312/cockatiel/issues/27))
+
 ## 1.1.0 - 2020-07-08
@@ -4,0 +8,0 @@

dist/common/Event.js

@@ -41,2 +41,6 @@ "use strict";
         return new Promise((resolve, reject) => {
+            const d2 = Event.once(event, data => {
+                d1.dispose();
+                resolve(data);
+            });
             const d1 = Event.once(cancellation.onCancellationRequested, () => {
@@ -46,6 +50,2 @@ d2.dispose();
             });
-            const d2 = Event.once(event, data => {
-                d1.dispose();
-                resolve(data);
-            });
         });
@@ -52,0 +52,0 @@ };

dist/common/Executor.js

@@ -24,3 +24,3 @@ "use strict";
 class ExecuteWrapper {
-    constructor(errorFilter = () => false, resultFilter = () => true) {
+    constructor(errorFilter = () => false, resultFilter = () => false) {
         this.errorFilter = errorFilter;
@@ -27,0 +27,0 @@ this.resultFilter = resultFilter;

dist/esm/common/Event.js

@@ -39,2 +39,6 @@ import { TaskCancelledError } from '../errors/TaskCancelledError';
         return new Promise((resolve, reject) => {
+            const d2 = Event.once(event, data => {
+                d1.dispose();
+                resolve(data);
+            });
             const d1 = Event.once(cancellation.onCancellationRequested, () => {
@@ -44,6 +48,2 @@ d2.dispose();
             });
-            const d2 = Event.once(event, data => {
-                d1.dispose();
-                resolve(data);
-            });
         });
@@ -50,0 +50,0 @@ };

dist/esm/common/Executor.js

@@ -22,3 +22,3 @@ import { EventEmitter } from './Event';
 export class ExecuteWrapper {
-    constructor(errorFilter = () => false, resultFilter = () => true) {
+    constructor(errorFilter = () => false, resultFilter = () => false) {
         this.errorFilter = errorFilter;
@@ -25,0 +25,0 @@ this.resultFilter = resultFilter;

dist/esm/TimeoutPolicy.js

@@ -63,2 +63,3 @@ import { CancellationToken, CancellationTokenSource } from './CancellationToken';
         const context = { cancellation: cts.token, cancellationToken: cts.token };
+        const onCancelledListener = cts.token.onCancellationRequested(() => this.timeoutEmitter.emit());
         try {
@@ -68,11 +69,13 @@ if (this.strategy === TimeoutStrategy.Cooperative) {
             }
-            return await Promise.race([
-                this.executor.invoke(fn, context, cts.token).then(returnOrThrow),
+            return await this.executor
+                .invoke(async () => Promise.race([
+                Promise.resolve(fn(context, cts.token)),
                 cts.token.cancellation(cts.token).then(() => {
-                    this.timeoutEmitter.emit();
                     throw new TaskCancelledError(`Operation timed out after ${this.duration}ms`);
                 }),
-            ]);
+            ]))
+                .then(returnOrThrow);
         }
         finally {
+            onCancelledListener.dispose();
             cts.cancel();
@@ -79,0 +82,0 @@ clearTimeout(timer);

dist/esm/TimeoutPolicy.test.js

 import { expect } from 'chai';
+import { stub } from 'sinon';
 import { promisify } from 'util';
 import { CancellationTokenSource } from './CancellationToken';
-import { defer } from './common/defer';
 import { runInChild } from './common/util.test';
@@ -26,4 +26,4 @@ import { TaskCancelledError } from './errors/TaskCancelledError';
         const policy = Policy.timeout(5, TimeoutStrategy.Aggressive);
-        const verified = defer();
-        await expect(policy.execute(async ({ cancellation }) => {
+        let verified;
+        await expect(policy.execute(async ({ cancellation }) => (verified = (async () => {
             await delay(0);
@@ -33,6 +33,4 @@ expect(cancellation.isCancellationRequested).to.be.false;
             expect(cancellation.isCancellationRequested).to.be.true;
-            verified.resolve(undefined);
-            return 42;
-        })).to.eventually.be.rejectedWith(TaskCancelledError);
-        await verified.promise;
+        })()))).to.eventually.be.rejectedWith(TaskCancelledError);
+        await verified;
     });
@@ -45,3 +43,3 @@ it('does not unref by default', async () => {
     `);
-        expect(output).to.contain('Operation cancelled');
+        expect(output).to.contain('Operation timed out');
     });
@@ -73,3 +71,67 @@ it('unrefs as requested', async () => {
     });
+    describe('events', () => {
+        let onSuccess;
+        let onFailure;
+        let onTimeout;
+        let agg;
+        let coop;
+        beforeEach(() => {
+            onSuccess = stub();
+            onFailure = stub();
+            onTimeout = stub();
+            coop = Policy.timeout(2, TimeoutStrategy.Cooperative);
+            agg = Policy.timeout(2, TimeoutStrategy.Aggressive);
+            for (const p of [coop, agg]) {
+                p.onFailure(onFailure);
+                p.onSuccess(onSuccess);
+                p.onTimeout(onTimeout);
+            }
+        });
+        it('emits a success event (cooperative)', async () => {
+            await coop.execute(() => 42);
+            await delay(3);
+            expect(onSuccess).to.have.been.called;
+            expect(onFailure).to.not.have.been.called;
+            expect(onTimeout).to.not.have.been.called;
+        });
+        it('emits a success event (aggressive)', async () => {
+            await agg.execute(() => 42);
+            await delay(3);
+            expect(onSuccess).to.have.been.called;
+            expect(onFailure).to.not.have.been.called;
+            expect(onTimeout).to.not.have.been.called;
+        });
+        it('emits a timeout event (cooperative)', async () => {
+            coop.onTimeout(onTimeout);
+            await coop.execute(() => delay(3));
+            expect(onSuccess).to.have.been.called; // still returned a good value
+            expect(onTimeout).to.have.been.called;
+            expect(onFailure).to.not.have.been.called;
+        });
+        it('emits a timeout event (aggressive)', async () => {
+            await expect(agg.execute(() => delay(3))).to.be.rejectedWith(TaskCancelledError);
+            expect(onSuccess).to.not.have.been.called;
+            expect(onTimeout).to.have.been.called;
+            expect(onFailure).to.have.been.called;
+        });
+        it('emits a failure event (cooperative)', async () => {
+            await expect(coop.execute(() => {
+                throw new Error('oh no!');
+            })).to.be.rejected;
+            await delay(3);
+            expect(onSuccess).to.not.have.been.called;
+            expect(onTimeout).to.not.have.been.called;
+            expect(onFailure).to.have.been.called;
+        });
+        it('emits a failure event (aggressive)', async () => {
+            await expect(agg.execute(() => {
+                throw new Error('oh no!');
+            })).to.be.rejected;
+            await delay(3);
+            expect(onSuccess).to.not.have.been.called;
+            expect(onTimeout).to.not.have.been.called;
+            expect(onFailure).to.have.been.called;
+        });
+    });
 });
 //# sourceMappingURL=TimeoutPolicy.test.js.map

dist/TimeoutPolicy.js

@@ -65,2 +65,3 @@ "use strict";
         const context = { cancellation: cts.token, cancellationToken: cts.token };
+        const onCancelledListener = cts.token.onCancellationRequested(() => this.timeoutEmitter.emit());
         try {
@@ -70,11 +71,13 @@ if (this.strategy === TimeoutStrategy.Cooperative) {
             }
-            return await Promise.race([
-                this.executor.invoke(fn, context, cts.token).then(Executor_1.returnOrThrow),
+            return await this.executor
+                .invoke(async () => Promise.race([
+                Promise.resolve(fn(context, cts.token)),
                 cts.token.cancellation(cts.token).then(() => {
-                    this.timeoutEmitter.emit();
                     throw new TaskCancelledError_1.TaskCancelledError(`Operation timed out after ${this.duration}ms`);
                 }),
-            ]);
+            ]))
+                .then(Executor_1.returnOrThrow);
         }
         finally {
+            onCancelledListener.dispose();
             cts.cancel();
@@ -81,0 +84,0 @@ clearTimeout(timer);

dist/TimeoutPolicy.test.js

 "use strict";
 Object.defineProperty(exports, "__esModule", { value: true });
 const chai_1 = require("chai");
+const sinon_1 = require("sinon");
 const util_1 = require("util");
 const CancellationToken_1 = require("./CancellationToken");
-const defer_1 = require("./common/defer");
 const util_test_1 = require("./common/util.test");
@@ -28,4 +28,4 @@ const TaskCancelledError_1 = require("./errors/TaskCancelledError");
         const policy = Policy_1.Policy.timeout(5, TimeoutPolicy_1.TimeoutStrategy.Aggressive);
-        const verified = defer_1.defer();
-        await chai_1.expect(policy.execute(async ({ cancellation }) => {
+        let verified;
+        await chai_1.expect(policy.execute(async ({ cancellation }) => (verified = (async () => {
             await delay(0);
@@ -35,6 +35,4 @@ chai_1.expect(cancellation.isCancellationRequested).to.be.false;
             chai_1.expect(cancellation.isCancellationRequested).to.be.true;
-            verified.resolve(undefined);
-            return 42;
-        })).to.eventually.be.rejectedWith(TaskCancelledError_1.TaskCancelledError);
-        await verified.promise;
+        })()))).to.eventually.be.rejectedWith(TaskCancelledError_1.TaskCancelledError);
+        await verified;
     });
@@ -47,3 +45,3 @@ it('does not unref by default', async () => {
     `);
-        chai_1.expect(output).to.contain('Operation cancelled');
+        chai_1.expect(output).to.contain('Operation timed out');
     });
@@ -75,3 +73,67 @@ it('unrefs as requested', async () => {
     });
+    describe('events', () => {
+        let onSuccess;
+        let onFailure;
+        let onTimeout;
+        let agg;
+        let coop;
+        beforeEach(() => {
+            onSuccess = sinon_1.stub();
+            onFailure = sinon_1.stub();
+            onTimeout = sinon_1.stub();
+            coop = Policy_1.Policy.timeout(2, TimeoutPolicy_1.TimeoutStrategy.Cooperative);
+            agg = Policy_1.Policy.timeout(2, TimeoutPolicy_1.TimeoutStrategy.Aggressive);
+            for (const p of [coop, agg]) {
+                p.onFailure(onFailure);
+                p.onSuccess(onSuccess);
+                p.onTimeout(onTimeout);
+            }
+        });
+        it('emits a success event (cooperative)', async () => {
+            await coop.execute(() => 42);
+            await delay(3);
+            chai_1.expect(onSuccess).to.have.been.called;
+            chai_1.expect(onFailure).to.not.have.been.called;
+            chai_1.expect(onTimeout).to.not.have.been.called;
+        });
+        it('emits a success event (aggressive)', async () => {
+            await agg.execute(() => 42);
+            await delay(3);
+            chai_1.expect(onSuccess).to.have.been.called;
+            chai_1.expect(onFailure).to.not.have.been.called;
+            chai_1.expect(onTimeout).to.not.have.been.called;
+        });
+        it('emits a timeout event (cooperative)', async () => {
+            coop.onTimeout(onTimeout);
+            await coop.execute(() => delay(3));
+            chai_1.expect(onSuccess).to.have.been.called; // still returned a good value
+            chai_1.expect(onTimeout).to.have.been.called;
+            chai_1.expect(onFailure).to.not.have.been.called;
+        });
+        it('emits a timeout event (aggressive)', async () => {
+            await chai_1.expect(agg.execute(() => delay(3))).to.be.rejectedWith(TaskCancelledError_1.TaskCancelledError);
+            chai_1.expect(onSuccess).to.not.have.been.called;
+            chai_1.expect(onTimeout).to.have.been.called;
+            chai_1.expect(onFailure).to.have.been.called;
+        });
+        it('emits a failure event (cooperative)', async () => {
+            await chai_1.expect(coop.execute(() => {
+                throw new Error('oh no!');
+            })).to.be.rejected;
+            await delay(3);
+            chai_1.expect(onSuccess).to.not.have.been.called;
+            chai_1.expect(onTimeout).to.not.have.been.called;
+            chai_1.expect(onFailure).to.have.been.called;
+        });
+        it('emits a failure event (aggressive)', async () => {
+            await chai_1.expect(agg.execute(() => {
+                throw new Error('oh no!');
+            })).to.be.rejected;
+            await delay(3);
+            chai_1.expect(onSuccess).to.not.have.been.called;
+            chai_1.expect(onTimeout).to.not.have.been.called;
+            chai_1.expect(onFailure).to.have.been.called;
+        });
+    });
 });
 //# sourceMappingURL=TimeoutPolicy.test.js.map

package.json

 {
   "name": "cockatiel",
-  "version": "1.1.0",
+  "version": "1.1.1",
   "description": "A resilience and transient-fault-handling library that allows developers to express policies such as Backoff, Retry, Circuit Breaker, Timeout, Bulkhead Isolation, and Fallback in a fluent and thread-safe manner. Inspired by .NET Polly.",
@@ -5,0 +5,0 @@ "main": "dist/index.js",

readme.md

@@ -932,2 +932,4 @@ # Cockatiel
 
+In the "aggressive" timeout strategy, a timeout event will immediately preceed a failure event and promise rejection. In the cooperative timeout strategy, the timeout event is still emitted, _but_ the success or failure is determined by what the executed function throws or returns.
+
 ```ts
@@ -934,0 +936,0 @@ const listener = timeout.onTimeout(() => console.log('timeout was reached'));

No alert changes

Improved metrics

Total package byte prevSize

518016

increased by2.55%

Lines of code

7569

increased by1.75%

Number of lines in readme file

1117

increased by0.18%

No dependency changes