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

fix: preserve the type of failures that are instances of Error #13

Merged
merged 2 commits into from
Aug 17, 2023

Conversation

vecerek
Copy link
Contributor

@vecerek vecerek commented Aug 14, 2023

Fixes #12

@vecerek
Copy link
Contributor Author

vecerek commented Aug 14, 2023

@iamchanii what does the following code mean:

if (Cause.isAnnotatedType(result.cause) && Cause.isFailType(result.cause.cause)) {
  throw result.cause.cause.error;
}

and how would I be able to return an annotated type as an alternative to this fix?

@iamchanii
Copy link
Owner

iamchanii commented Aug 15, 2023

@iamchanii what does the following code mean:

if (Cause.isAnnotatedType(result.cause) && Cause.isFailType(result.cause.cause)) {
  throw result.cause.cause.error;
}

and how would I be able to return an annotated type as an alternative to this fix?

@vecerek At least when I implemented, Effect.runPromiseExit provided annotated exit.

EffectPrimitiveFailure {
  _tag: 'Failure',
  i0: {
    _tag: 'Annotated',
    cause: { _tag: 'Fail', error: [NotFound] },
    annotation: StackAnnotation {
      stack: [Object],
      seq: 1,
      [Symbol(@effect/io/Cause/StackAnnotation)]: Symbol(@effect/io/Cause/StackAnnotation)
    }
  },
  i1: undefined,
  i2: undefined,
  trace: undefined,
  [Symbol(@effect/io/Effect)]: { _R: [Function: _R], _E: [Function: _E], _A: [Function: _A] }
}

But now we got just Failed exit. so Cause.isAnnotatedType(...) condition is false.

EffectPrimitiveFailure {
  _tag: 'Failure',
  i0: {
    _tag: 'Fail',
    error: NotFound: User iamchanii2 not found
        at <anonymous> (/Users/iamchanii/dev/pothos-plugin-effect/examples/02-with-errors-plugin/src/GitHub.ts:51:63)
        at e (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/effect.ts:250:14)
        at cause (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/core.ts:683:16)
        at FiberRuntime.Failure (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/fiberRuntime.ts:1082:25)
        at <anonymous> (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/fiberRuntime.ts:1278:51)
        at onRun (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/supervisor.ts:227:12)
        at runLoop (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/fiberRuntime.ts:1276:32)
        at evaluateEffect (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/fiberRuntime.ts:846:29)
        at evaluateMessageWhileSuspended (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/fiberRuntime.ts:813:14)
        at drainQueueOnCurrentThread (/Users/iamchanii/dev/pothos-plugin-effect/node_modules/.pnpm/@effect+io@0.37.1/node_modules/@effect/io/src/internal/fiberRuntime.ts:569:18) {
      _tag: 'NotFound'
    }
  },
  i1: undefined,
  i2: undefined,
  trace: undefined,
  [Symbol(@effect/io/Effect)]: { _R: [Function: _R], _E: [Function: _E], _A: [Function: _A] }
}

It means Effect.runPromiseExit's internal implementation in @effect/io was changed from v0.30.0 to now.

src/field-builder.ts Outdated Show resolved Hide resolved
Comment on lines 69 to 71
if (Cause.isFailType(result.cause) && (result.cause.error as any instanceof Error)) {
throw result.cause.error;
}
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

result.cause.error could can be not an instance of Error. so it should be wraped by failErrorConstructor, effectOptions.defaultFailErrorConstructor or Error.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be misunderstanding you but that's exactly the reason for which I'm also checking if result.cause.error instanceof Error in the above code. If it isn't, I want it to fall through and be handled by the throw logic below:

throw new (effect.failErrorConstructor ?? effectOptions?.defaultFailErrorConstructor ?? Error)(
  Cause.pretty(result.cause),
);

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahh yep. you're right.

@iamchanii iamchanii merged commit 0bc23e3 into iamchanii:main Aug 17, 2023
@iamchanii
Copy link
Owner

fyi: there might be situated that result.cause is annotated, so I use Cause.unannotate a5c9a89

Ref: https://canary.discord.com/channels/795981131316985866/1141586320624468068

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

Successfully merging this pull request may close these issues.

Bug: type of the error failure is not preserved
2 participants