gcanti / fp-ts

Functional programming in TypeScript
https://gcanti.github.io/fp-ts/
MIT License
10.84k stars 503 forks source link

Suggestion: improve stack traces with explicit function names #1431

Open OliverJAsh opened 3 years ago

OliverJAsh commented 3 years ago

🚀 Feature request

Current Behavior

When an exception happens inside of an operator, the stack trace is ambiguous: it does not say which operator the exception occurred inside of.

const { pipe } = require("fp-ts/function");
const O = require("fp-ts/Option");

const as = O.some(1);
const fn = () => {
  throw new Error("oops");
};
pipe(as, O.map(fn));
Error: oops
    at fn (/Users/oliverash/Development/fp-ts-stack-trace-test/Option.js:6:9)
    at /Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/Option.js:411:61
    at pipe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/function.js:188:20)
    at Object.<anonymous> (/Users/oliverash/Development/fp-ts-stack-trace-test/Option.js:8:1)

Observe the second entry in the stack:

    at /Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/Option.js:411:61

This entry represents the function returned by map, which is an anonymous function with no name:

//                 ⬇ this function
const map = (f) => (fa) => {
  return isNone(fa) ? none : some(f(fa.value));
};

Nothing in the stack trace tells you that this exception occurred during the execution of map.

Desired Behavior

When an exception happens inside of an operator, the stack trace should not be ambiguous: it should say which operator the exception occurred inside of, similar to Array.prototype.map:

const as = [1, 2, 3];
const fn = () => {
  throw new Error("oops");
};
as.map(fn);
Error: oops
    at fn (/Users/oliverash/Development/fp-ts-stack-trace-test/Array.js:3:9)
    at Array.map (<anonymous>)
    at Object.<anonymous> (/Users/oliverash/Development/fp-ts-stack-trace-test/Array.js:5:4)

Observe the second entry in the stack:

    at Array.map (<anonymous>)

Suggested Solution

We can achieve similar behaviour with fp-ts behaviours if we provide explicit function names for the functions returned by operators such as map:

const map = (f) => {
  const fn = (fa) => {
    return isNone(fa) ? none : some(f(fa.value));
  };
  Object.defineProperty(fn, "name", { value: "Option.map" });
  return fn;
};

The stack trace now appears like so:

 Error: oops
     at fn (/Users/oliverash/Development/fp-ts-stack-trace-test/Option-new.js:20:9)
-    at /Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/Option.js:411:61
+    at Option.map (/Users/oliverash/Development/fp-ts-stack-trace-test/Option-new.js:12:37)
     at pipe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/function.js:188:20)
     at Object.<anonymous> (/Users/oliverash/Development/fp-ts-stack-trace-test/Option-new.js:22:1)

Alternatively:

const map = (f) => function optionMap(fa) {
  return isNone(fa) ? none : some(f(fa.value));
};
 Error: oops
     at fn (/Users/oliverash/Development/fp-ts-stack-trace-test/Option-new.js:20:9)
-    at /Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/Option.js:411:61
+    at optionMap (/Users/oliverash/Development/fp-ts-stack-trace-test/Option-new.js:12:37)
     at pipe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/fp-ts/lib/function.js:188:20)
     at Object.<anonymous> (/Users/oliverash/Development/fp-ts-stack-trace-test/Option-new.js:22:1)

Full example for testing:

const { pipe } = require("fp-ts/function");

const none = { _tag: "None" };
const some = (a) => {
  return { _tag: "Some", value: a };
};
const isNone = (fa) => {
  return fa._tag === "None";
};
const map = (f) => {
  const fn = (fa) => {
    return isNone(fa) ? none : some(f(fa.value));
  };
  Object.defineProperty(fn, "name", { value: "Option.map" });
  return fn;
};

const as = some(1);
const fn = () => {
  throw new Error("oops");
};
pipe(as, map(fn));

Who does this impact? Who is this for?

All users who care about the debugging experience when exceptions inevitably happen.

Describe alternatives you've considered

Additional context

Your environment

Software Version(s)
fp-ts
TypeScript
OliverJAsh commented 3 years ago

Interestingly RxJS (which also uses pipeable operators) doesn't suffer from the same problem, presumably because each operator is defined as a class instance. Not sure we'd want that though.

const { pipe } = require("fp-ts/function");
const Rx = require("rxjs");
const RxO = require("rxjs/operators");

const as = Rx.of(1);
const fn = () => {
  throw new Error("oops");
};
pipe(as, RxO.map(fn)).subscribe();
Error: oops
    at MapSubscriber.fn [as project] (/Users/oliverash/Development/fp-ts-stack-trace-test/Rx.js:7:9)
    at MapSubscriber._next (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/operators/map.js:49:35)
    at MapSubscriber.Subscriber.next (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/Subscriber.js:66:18)
    at Observable._subscribe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/util/subscribeToArray.js:5:20)
    at Observable._trySubscribe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/Observable.js:44:25)
    at Observable.subscribe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/Observable.js:30:22)
    at MapOperator.call (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/operators/map.js:32:23)
    at Observable.subscribe (/Users/oliverash/Development/fp-ts-stack-trace-test/node_modules/rxjs/internal/Observable.js:25:31)
    at Object.<anonymous> (/Users/oliverash/Development/fp-ts-stack-trace-test/Rx.js:9:23)
OliverJAsh commented 3 years ago

Side note: I know exceptions are discouraged in FP, but inevitably they can still happen, e.g. if the types are wrong or you call a function which you didn't realise would throw.

mikearnaldi commented 3 years ago

I would prefer using named functions function x() instead of const x = () => so that the change wouldn't hurt the readability of code but, in principle, I am not opposed to improving traces even though this should be considered a precautionary measure rather than a standard (APIs that throw should be wrapped in things like IOEither)

jleider commented 3 years ago

One concern would be to make sure the non-exported named functions dont get mangled by minification. I think by default non top level functions get mangled which would lead to no real gain here, right? Also, don't source maps provide you this information?

OliverJAsh commented 3 years ago

don't source maps provide you this information?

Hmm, source maps can't help if the function doesn't have a name. Reduced test case:

const map = () => () => 1;
map().name // => ''

One concern would be to make sure the non-exported named functions dont get mangled by minification.

That won't be an issue as long as you're using source maps.