Skip to content

Commit

Permalink
feat(data-point): add reducer stack trace functionality (ViacomInc#409)
Browse files Browse the repository at this point in the history
Property `error.reducerStackTrace` now contains a stack trace of all reducers executed before and including the reducer with the error.
  • Loading branch information
acatl authored Aug 31, 2019
1 parent 8728c29 commit 23cf880
Show file tree
Hide file tree
Showing 4 changed files with 106 additions and 16 deletions.
21 changes: 19 additions & 2 deletions packages/data-point/src/Accumulator.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,9 +30,9 @@ class Accumulator {
this.tracer = options.tracer;

/**
* @type {Reducer} reference to the current reducer
* @type {string[]} stores the list of reducers executed per branch.
*/
this.reducer = options.reducer;
this.__reducerStackTrace = [];

/**
* @type {Number} current reducer process id, this number changes on the
Expand All @@ -59,6 +59,23 @@ class Accumulator {
return copy;
}

set reducer(reducer) {
this.__reducer = reducer;

// we create a new array to create a new reducer history branch.
this.__reducerStackTrace = this.__reducerStackTrace.concat(
this.__reducer.id
);
}

get reducer() {
return this.__reducer;
}

get reducerStackTrace() {
return this.__reducerStackTrace.join(" > ");
}

resolve(reducers) {
return this.__resolve(this, reducers);
}
Expand Down
43 changes: 41 additions & 2 deletions packages/data-point/src/Accumulator.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,15 +5,16 @@ describe("Accumulator", () => {
it("should accept empty options object", () => {
expect(new Accumulator()).toMatchInlineSnapshot(`
Accumulator {
"__reducerStackTrace": Array [],
"cache": Object {},
"locals": undefined,
"pid": undefined,
"reducer": undefined,
"tracer": undefined,
"value": undefined,
}
`);
});

it("should assign options object", () => {
const resolve = () => true;
const acc = new Accumulator({
Expand All @@ -23,12 +24,13 @@ describe("Accumulator", () => {
tracer: "tracer",
resolve
});

expect(acc).toMatchInlineSnapshot(`
Accumulator {
"__reducerStackTrace": Array [],
"cache": "cache",
"locals": "locals",
"pid": undefined,
"reducer": undefined,
"tracer": "tracer",
"value": "value",
}
Expand Down Expand Up @@ -60,6 +62,43 @@ describe("Accumulator", () => {
expect(Object.getPrototypeOf(acc2)).toEqual(acc1);
});
});

describe("reducer", () => {
const reducer = {
id: "myReducerId"
};

describe("set reducer", () => {
it("should append a new reducer to the stack of reducers", () => {
const acc = new Accumulator({
value: "a"
});

// eslint-disable-next-line no-underscore-dangle
const originalReducerStackTrace = acc.__reducerStackTrace;

acc.reducer = reducer;

// should not mutate the original stackTrace
expect(originalReducerStackTrace).toEqual([]);
// eslint-disable-next-line no-underscore-dangle
expect(acc.__reducerStackTrace).toEqual(["myReducerId"]);
});
});

describe("get reducer", () => {
it("should return stored internal reducer", () => {
const acc = new Accumulator({
value: "a"
});

acc.reducer = reducer;
// eslint-disable-next-line no-underscore-dangle
expect(acc.__reducer).toEqual(reducer);
});
});
});

describe("resolve", () => {
it("should call internal resolve method", () => {
const mockResolve = jest.fn(() => "resolved");
Expand Down
8 changes: 8 additions & 0 deletions packages/data-point/src/resolve.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,15 @@ async function resolve(accumulator, reducer) {
// NOTE: recursive call by passing resolve method
result = await reducer.resolveReducer(acc, resolve);
} catch (error) {
// because the error bubbles up, we only want to set `error.reducerStackTrace`
// once, otherwise we end up with only the first item in the stack.
if (!error.reducerStackTrace) {
error.reducerStackTrace = acc.reducerStackTrace;
}

traceSpan.logError(span, error);

// rethrow the error up
throw error;
} finally {
traceSpan.finish(span);
Expand Down
50 changes: 38 additions & 12 deletions packages/data-point/src/resolve.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,41 @@ describe("resolve", () => {
});
});

describe("reducerStackTrace", () => {
it("should augment error with reducer stacktrace", async () => {
const acc = new Accumulator();

const testError = new Error("resolveReducer failed");

const reducerRejected = {
id: "badReducer",
resolveReducer: jest.fn().mockRejectedValue(testError)
};

const result = await resolve(acc, reducerRejected).catch(error => error);

expect(result).toBeInstanceOf(Error);
expect(result.reducerStackTrace).toEqual("badReducer");
});

it("should not augment error with reducer stacktrace if already set", async () => {
const acc = new Accumulator();

const testError = new Error("resolveReducer failed");
testError.reducerStackTrace = "initialReducerStackTrace";

const reducerRejected = {
id: "badReducer",
resolveReducer: jest.fn().mockRejectedValue(testError)
};

const result = await resolve(acc, reducerRejected).catch(error => error);

expect(result).toBeInstanceOf(Error);
expect(result.reducerStackTrace).toEqual("initialReducerStackTrace");
});
});

describe("tracing", () => {
it("should call traceSpan.create with accumulator", async () => {
const acc = new Accumulator();
Expand All @@ -49,25 +84,16 @@ describe("resolve", () => {
it("should call traceSpan.logError when resolveReducer throws error", async () => {
const acc = new Accumulator();

const testError = new Error("resolveReducer failed");
const reducerRejected = {
resolveReducer: jest
.fn()
.mockRejectedValue(new Error("resolveReducer failed"))
resolveReducer: jest.fn().mockRejectedValue(testError)
};

await resolve(acc, reducerRejected).catch(error => error);

expect(traceSpan.logError).toBeCalled();

// first argument would is an undefined span
expect(traceSpan.logError.mock.calls).toMatchInlineSnapshot(`
Array [
Array [
undefined,
[Error: resolveReducer failed],
],
]
`);
expect(traceSpan.logError.mock.calls[0]).toEqual([undefined, testError]);
});

it("should finally call traceSpan.finish()", async () => {
Expand Down

0 comments on commit 23cf880

Please # to comment.