Is the signature of TaskEither.tapError as intended?

[aknauf]

🐛 Bug report

Not sure if I would call this a bug; perhaps I misunderstand the purpose? My understanding is that tapError should allow me to peek at the error without affecting the result, but it seems that maintaining the signature requires a superfluous return TE.of(<basically anything>)? Surely any return value here is unnecessary?

Perhaps this is really a feature request for a convenience overload?

Or perhaps I'm missing some existing functionality?

Current Behavior

The argument to TE.tapError must return a TaskEither. The value returned by the tap affects the value returned by the flow.

Expected behavior

The argument to tapError should not require any return value. It's return should not affect the flow.

Reproducible example

import * as TE from 'fp-ts/TaskEither'
import {flow} from 'fp-ts/function'
import * as E from "fp-ts/Either";

const log = jest.fn();

const getStuff = async (fail: boolean = false): Promise<string> => {
    if (fail) throw new Error('Oh no');

    return 'stuff';
}

const getStuffFunctionally = flow(
    TE.tryCatchK(getStuff, E.toError),
    TE.tapError(e => {
        log(`This is not good. ${e.message}`);
        return TE.of(e); // should this really be needed?
    })
)

describe('tapError', () => {
    it('should get stuff', async () => {
        await expect(getStuffFunctionally()()).resolves.toEqual(E.right('stuff'));
    })

    it('should give error on failure', async () => {
        await expect(getStuffFunctionally(true)()).resolves.toEqual(E.left(new Error('Oh no')))
    });

    it('should log on failure', async () => {
        await getStuffFunctionally(true)();

        expect(log).toHaveBeenCalledWith('This is not good. Oh no');
    });
});

Suggested solution(s)

TE.tapError could do the necessary wrapping/unwrapping. Ideally, I could write TE.tapError(log).

Additional context

Your environment

Which versions of fp-ts are affected by this issue? Did this work in previous versions of fp-ts?

Software	Version(s)
fp-ts	2.16.1
TypeScript	5.2.2

[SilentEchoGM]

If you don't need to keep the error, TE.mapError without a return will work fine, if you do need to keep the error then you can use TE.mapError(trace(message)) where trace is:

export const trace =
  (label: string) =>
  <T>(value: T) => {
    console.log(label, value);
    return value;
  };

This should explain tap functions a little better for you: https://gcanti.github.io/fp-ts/modules/TaskEither.ts.html#tap

I don't think I've ever noticed a need for a tap function, I probably just inlined a specific lambda that did the same!