Skip to content

Commit

Permalink
fix: ctx is copied to avoid mutation side effects
Browse files Browse the repository at this point in the history
This was causing a peculiar bug where if a `ctx` was passed to function calls twice in a method it could cause the signal abortion chain to be broken. In essence in stead of branching the chain out with multiple call paths. The mutation flattened the tree into a chain, when one branch completed the decorator clean up broke the abortion chain.
  • Loading branch information
tegefaulkes committed Oct 7, 2022
1 parent aae353f commit 0a2653a
Show file tree
Hide file tree
Showing 6 changed files with 36 additions and 12 deletions.
6 changes: 5 additions & 1 deletion src/contexts/decorators/cancellable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,12 @@ function cancellable(lazy: boolean = false) {
let ctx: Partial<ContextCancellable> = args[contextIndex];
if (ctx === undefined) {
ctx = {};
args[contextIndex] = ctx;
} else {
// Copy the ctx into a new ctx object to avoid mutating the ctx in case
// it is used again
ctx = { ...ctx };
}
args[contextIndex] = ctx;
// Runtime type check on the context parameter
contextsUtils.checkContextCancellable(ctx, key, targetName);
return setupCancellable(
Expand Down
24 changes: 20 additions & 4 deletions src/contexts/decorators/timed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ function timed(
let ctx: Partial<ContextTimed> = args[contextIndex];
if (ctx === undefined) {
ctx = {};
args[contextIndex] = ctx;
} else {
// Copy the ctx into a new ctx object to avoid mutating the ctx in case
// it is used again
ctx = { ...ctx };
}
args[contextIndex] = ctx;
// Runtime type check on the context parameter
contextsUtils.checkContextTimed(ctx, key, targetName);
const teardownContext = setupTimedContext(
Expand All @@ -51,8 +55,12 @@ function timed(
let ctx: Partial<ContextTimed> = args[contextIndex];
if (ctx === undefined) {
ctx = {};
args[contextIndex] = ctx;
} else {
// Copy the ctx into a new ctx object to avoid mutating the ctx in case
// it is used again
ctx = { ...ctx };
}
args[contextIndex] = ctx;
// Runtime type check on the context parameter
contextsUtils.checkContextTimed(ctx, key, targetName);
const teardownContext = setupTimedContext(
Expand All @@ -71,8 +79,12 @@ function timed(
let ctx: Partial<ContextTimed> = args[contextIndex];
if (ctx === undefined) {
ctx = {};
args[contextIndex] = ctx;
} else {
// Copy the ctx into a new ctx object to avoid mutating the ctx in case
// it is used again
ctx = { ...ctx };
}
args[contextIndex] = ctx;
// Runtime type check on the context parameter
contextsUtils.checkContextTimed(ctx, key, targetName);
const teardownContext = setupTimedContext(
Expand All @@ -91,8 +103,12 @@ function timed(
let ctx: Partial<ContextTimed> = args[contextIndex];
if (ctx === undefined) {
ctx = {};
args[contextIndex] = ctx;
} else {
// Copy the ctx into a new ctx object to avoid mutating the ctx in case
// it is used again
ctx = { ...ctx };
}
args[contextIndex] = ctx;
// Runtime type check on the context parameter
contextsUtils.checkContextTimed(ctx, key, targetName);
const teardownContext = setupTimedContext(
Expand Down
6 changes: 5 additions & 1 deletion src/contexts/decorators/timedCancellable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,12 @@ function timedCancellable(
let ctx: Partial<ContextTimed> = args[contextIndex];
if (ctx === undefined) {
ctx = {};
args[contextIndex] = ctx;
} else {
// Copy the ctx into a new ctx object to avoid mutating the ctx in case
// it is used again
ctx = { ...ctx };
}
args[contextIndex] = ctx;
// Runtime type check on the context parameter
contextsUtils.checkContextTimed(ctx, key, targetName);
const lazy_ = typeof lazy === 'boolean' ? lazy : lazy(this);
Expand Down
2 changes: 1 addition & 1 deletion src/contexts/functions/cancellable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ function cancellable<C extends ContextCancellable, P extends Array<any>, R>(
lazy: boolean = false,
): (...params: ContextAndParameters<C, P>) => PromiseCancellable<R> {
return (...params) => {
const ctx = params[0] ?? {};
const ctx = params[0] != null ? { ...params[0] } : {};
const args = params.slice(1) as P;
return setupCancellable(f, lazy, ctx, args);
};
Expand Down
8 changes: 4 additions & 4 deletions src/contexts/functions/timed.ts
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ function timed<C extends ContextTimed, P extends Array<any>>(
): (...params: ContextAndParameters<C, P>) => any {
if (f instanceof utils.AsyncFunction) {
return async (...params) => {
const ctx = params[0] ?? {};
const ctx = params[0] != null ? { ...params[0] } : {};
const args = params.slice(1) as P;
const teardownContext = setupTimedContext(
delay,
Expand All @@ -140,7 +140,7 @@ function timed<C extends ContextTimed, P extends Array<any>>(
};
} else if (f instanceof utils.GeneratorFunction) {
return function* (...params) {
const ctx = params[0] ?? {};
const ctx = params[0] != null ? { ...params[0] } : {};
const args = params.slice(1) as P;
const teardownContext = setupTimedContext(
delay,
Expand All @@ -155,7 +155,7 @@ function timed<C extends ContextTimed, P extends Array<any>>(
};
} else if (f instanceof utils.AsyncGeneratorFunction) {
return async function* (...params) {
const ctx = params[0] ?? {};
const ctx = params[0] != null ? { ...params[0] } : {};
const args = params.slice(1) as P;
const teardownContext = setupTimedContext(
delay,
Expand All @@ -170,7 +170,7 @@ function timed<C extends ContextTimed, P extends Array<any>>(
};
} else {
return (...params) => {
const ctx = params[0] ?? {};
const ctx = params[0] != null ? { ...params[0] } : {};
const args = params.slice(1) as P;
const teardownContext = setupTimedContext(
delay,
Expand Down
2 changes: 1 addition & 1 deletion src/contexts/functions/timedCancellable.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ function timedCancellable<C extends ContextTimed, P extends Array<any>, R>(
errorTimeoutConstructor: new () => Error = contextsErrors.ErrorContextsTimedTimeOut,
): (...params: ContextAndParameters<C, P>) => PromiseCancellable<R> {
return (...params) => {
const ctx = params[0] ?? {};
const ctx = params[0] != null ? { ...params[0] } : {};
const args = params.slice(1) as P;
return setupTimedCancellable(
f,
Expand Down

0 comments on commit 0a2653a

Please sign in to comment.