Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Is the signature of TaskEither.tapError as intended? #1914

Open
aknauf opened this issue Oct 25, 2023 · 1 comment
Open

Is the signature of TaskEither.tapError as intended? #1914

aknauf opened this issue Oct 25, 2023 · 1 comment

Comments

@aknauf
Copy link

aknauf commented Oct 25, 2023

🐛 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
Copy link

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!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants