Code Monkey home page Code Monkey logo

Comments (13)

mikearnaldi avatar mikearnaldi commented on May 28, 2024 2

There seems to be some confusion around context/layer/services, they are different things:

  1. a Layer is Constructor of Context, a Layer should only be built at boot time
  2. a Context is an Object that carries a set of services
  3. a Service is a value contained in a Context object

Generally speaking "field-level" provision of layer / context is useless, the sole reason to use context is to let the context bubble up through your app and provide it at the top, putting in the same place access of context and provision of context defeats the purpose of using context in the first place, a user could simply do whateverEffect.pipe(Effect.provide(context)) by themselves, no new api needed.

You really have only two choices:

  • you control initialisation and expose it as an effect itself so your final composed schema exposes some kind of "bindApp: Effect<ContextFromSchema, never, void>" and you internally run an Effect.runtime() during init to get a runtime to use when resolving queries (that carries context)
  • you don't control initialisation and rely on a user giving you a runtime that you use to execute

In a general sense to use Effect you need to know about Runtime so I wouldn't worry about a user that wants to use Effect but doesn't know Runtime

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024 1

It looks ugly but working.

  builder = new SchemaBuilder<SchemaTypes>({
    effectOptions: {
      globalLayer: Layer.provide(
        Effect.setConfigProvider(ConfigProvider.fromMap(new Map([['HOST', 'localhost']]))),
        Layer.context(),
      ),
    },
    plugins: [EffectPlugin, RelayPlugin],
    relayOptions: {},
  });

  builder.queryType({});

  builder.queryField('roll', t =>
    t.effect({
      resolve() {
        return pipe(
          Effect.config(Config.withDefault(Config.string('HOST'), '0.0.0.0')),
          Effect.flatMap(host => Effect.succeed(host)),
          // What should I do for ConfigError?
          Effect.catchAll(() => Effect.succeed('none')),
        );
      },
      type: 'String',
    }));

  const schema = builder.toSchema();
  const document = parse(`{ roll }`);
  const result = await execute({ document, schema });

  expect(result.data).toEqual({
    roll: 'localhost',
  });

What does it type checks but does not seem to work at all meaning? I just feeling I misunderstood.

from pothos-plugin-effect.

vecerek avatar vecerek commented on May 28, 2024 1

@iamchanii I think that looks good as a high-level plan

Regarding the details of:

declare const effectFieldResult: Effect<...>;

return Runtime.runPromise(effectOptions.runtime || Runtime.defaultRuntime)(effectFieldResult);

In the end, I still had to unwrap the error so that the pothos error plugin keeps working like so:

declare const effectFieldResult: Effect<...>;

return pipe(
  effectFieldResult,
  Runtime.runPromiseExit(effectOptions.runtime)
).then((exit) => {
  if (Exit.isFailure(exit)) {
    const cause = Cause.originalError(exit.cause);

    if (Cause.isFailType(cause) && cause.error instanceof Error) {
      throw cause.error;
    }

    throw new Error(Cause.pretty(cause));
  }

  return exit.value;
})

from pothos-plugin-effect.

XiNiHa avatar XiNiHa commented on May 28, 2024 1

(fyi; failErrorConstructor was implemented from #2 and I think it is very rare use case. I'm considering offering it only as a plugin-level options. not field. cc. @XiNiHa )

Although I agree that the customization ability is very rare to use, throwing the error cause as-is completely breaks the plugin behavior. Therefore I believe the plugin should expose a custom error type (like EffectFailureError?) to throw with a non-error cause.

from pothos-plugin-effect.

vecerek avatar vecerek commented on May 28, 2024

@iamchanii I have a bit more complex setup than your example. I tried your example and it sort of works. In my case, I am providing a layer to the effect field in its configuration. That layer depends on another layer, which depends on an another layer until the very last layer actually depends on a config. For some reason, I'm getting an error like this:

(Missing data at /secrets.MY_SECRET: "Expected /secrets_MY_SECRET to exist in the process context")

However, I'm providing a config provider from a map and therefore I'm expecting to see something more like this:

(Missing data at /secrets.MY_SECRET: "Expected /secrets.MY_SECRET to exist in the provided map")

This is confusing to me but irrelevant for the big picture. I'll try to water my use case down to a minimal reproducible code. However, my bigger concern is the following.

I would like this error to happen at deploy time instead of at runtime, i.e. when resolving the GraphQL requests. I want my application to fail the deployment if some configuration is missing. I'm not sure how this could be achieved, or if at all in this plugin but it's a crucial requirement I have nonetheless. The problem I see is that pothos-plugin-effect creates an effect runtime on every request. Somehow, I'd need that runtime to be built ahead of time and the resolution of all graphql requests would need to live within that single runtime. That would solve my issue, I believe. I just don't know how feasible that is.

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024

I would like this error to happen at deploy time instead of at runtime, i.e. when resolving the GraphQL requests. I want my application to fail the deployment if some configuration is missing

In fact, there is very basic type inferrence for effect layers, contexts and services. getting layers as typed tuple and typing it is little bit difficult (at least for me) but I think this library have to do it.

The problem I see is that pothos-plugin-effect creates an effect runtime on every request. Somehow, I'd need that runtime to be built ahead of time and the resolution of all graphql requests would need to live within that single runtime.

Interest. How about this api?

effectOptions: {
  customRuntime: () => {
    return Runtime.make(...);
  };
}

from pothos-plugin-effect.

vecerek avatar vecerek commented on May 28, 2024

Hey @iamchanii 👋 sorry for coming back to you so late. I was thinking about this plugin a lot these past few weeks. The whole layer thing didn't work for me conceptually. I believe it is an API design mistake to surface the construction of layers through the field definitions. In Effect, it is important that these layers get constructed when the application starts as opposed to at runtime. As an application author, I do not want to handle ConfigErrors at runtime. I want the deployment of my application to fail if the configuration is wrong or missing. Otherwise, such failures could lead to serious production incidents which could have been easily avoided.

To figure out what the core value of this plugin is, I decided to remove it from my codebase and try to use some minimal implementation. This is what I ended up with (it's just pseudo-code, don't mind if it doesn't run):

When I initialize my app, I create an Effect runtime which I pass to the graphql resolvers via a modified context object.

// index.ts
const main = async () => {
  const runtime = await makeRuntime(); // imagine this returns an Effect runtime

  const yoga = createYoga({
    schema,
    (context) => ({ ...context, runtime }),
  });

  const server = createServer(yoga);

  server.listen(4000, () => {
    console.info('Server is running on http://localhost:4000/graphql');
  });
}

void main().catch(console.error);

I also created what I think the core behavior of this library is, a graphql effect resolver:

export const resolveEffect = <R>(
  runtime: Runtime.Runtime<R>
) => async <E, A>(
  effect: Effect.Effect<R, E, A>
) => {
  const exit = await Runtime.runPromiseExit(runtime)(effect);

  if (Exit.isFailure(exit)) {
    const cause = Cause.unannotate(exit.cause);

    if (Cause.isFailType(cause) && cause.error instanceof Error) {
      throw cause.error;
    }

    throw new Error(Cause.pretty(cause));
  }

  return exit.value;
}

And then, all I need to do is use it in my resolver like so:

resolver: (_parent, _args, context) => pipe(
  // imagine there's an Effect here,
  resolveEffect(context.runtime)
)

Having reflected on this issue, I've come to the conclusion that this plugin should do two things:

  1. remove all mentions of layers to prevent its users from shooting themselves in the foot,
  2. allow the user to pass in a reference to a runtime, not a runtime construction function, to make sure that this runtime is built at application startup time, not during the handling of a request. It should be a single runtime reference passed to all resolvers. Then the plugin could do the effect resolution for me as it does today but using the runtime if one was provided.

What do you think @iamchanii?

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024

@vecerek I'm so appreciate for your feedback! first of all, this library started from just small idea so there may be design mistakes. but I think we can still take it right way. and then, we can release first major release.

I'm not sure if you know, recently I worked about refactoring and writing some documentations. and I got some thoughts as similar as you thought. Is really need to define services, contexts and layers in object field? and I am not experienced about Effect, but in general, I think it's enoguth to have a "main" Layer to context management. (And, hmm. I still think field-level context management maybe usefull some use cases nevertheless)

remove all mentions of layers to prevent its users from shooting themselves in the foot,

👍

Is this meaning for field-level's mentions of layers? just applied for layers? how about contexts and services?

allow the user to pass in a reference to a runtime, not a runtime construction function, to make sure that this runtime is built at application startup time, not during the handling of a request. It should be a single runtime reference passed to all resolvers. Then the plugin could do the effect resolution for me as it does today but using the runtime if one was provided.

👍

like so:

// $ExpectType Runtime.Runtime<...>
const runtime = await makeEffectRuntime();

const builder = new SchemaBuilder<{
  EffectRuntime: typeof runtime
  // or
  // EffectRuntime: Runtime.Runtime<...>
}>({
  plugins: [EffectPlugin],
  effectOptions: { runtime },
});

I think this is more simple than globalLayer and globalContext and this could be replaced these features. but I concerned that all users of this plugin will require knowledges about Runtime and make it. how do you think about it?

from pothos-plugin-effect.

vecerek avatar vecerek commented on May 28, 2024

I fully agree with @mikearnaldi. Because of the design of pothos, controlling the initialization of the app is not achievable. Our only option is:

you don't control initialisation and rely on a user giving you a runtime that you use to execute

And about

but I concerned that all users of this plugin will require knowledges about Runtime and make it. how do you think about it?

I don't think Runtime is a hard-to-grasp concept. Also, if users already have a Layer (could be one that was created by merging many layers), creating a Runtime from that layer is as simple as applying Layer.toRuntime. Providing some code examples that show how runtimes can be created should be enough to address your concerns.

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024

@mikearnaldi @vecerek thank you for your comments! I'm still learning about Effect, so I don't have it all figured out, but I'll share some thoughts on the plugin's implementation soon.

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024

I just want sync my thoughts, and want to get a review from you.

  1. Remove effect options from t.effect. and then, I would be able to remove many lines of type inference related code.

    t.effect({
      type: 'String',
    - effect: {
    -   layers: [/* ... */],
    -   contexts: [/* ... */],
    -   services: [/* ... */],
    -   failErrorConstructor: Error
    - }
    })

    (fyi; failErrorConstructor was implemented from #2 and I think it is very rare use case. I'm considering offering it only as a plugin-level options. not field. cc. @XiNiHa )

  2. When constructing SchemaBuilder, it will be accept runtime that the user wants to use globally. so, plugin doesn't control the runtime initialization and checking it is not the plugin's concern.

    const builder = new SchemaBuilder({
      plugins: [EffectPlugin],
      effectOptions: {
    +   runtime: appRuntime
    +   // runtime: async (context) => appRuntime
      }
    });

    t.effect field resolver will do the following things:

    declare const effectFieldResult: Effect<...>;
    
    return pipe(
      effectFieldResult,
      Runtime.runPromiseExit(effectOptions.runtime)
    ).then((exit) => {
      if (Exit.isFailure(exit)) {
        const cause = Cause.originalError(exit.cause);
    
        if (Cause.isFailType(cause) && cause.error instanceof Error) {
          throw cause.error;
        }
    
        throw new EffectFailureError(Cause.pretty(cause), { cause })
      }
    
      return exit.value;
    })

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024

(fyi; failErrorConstructor was implemented from #2 and I think it is very rare use case. I'm considering offering it only as a plugin-level options. not field. cc. @XiNiHa )

Although I agree that the customization ability is very rare to use, throwing the error cause as-is completely breaks the plugin behavior. Therefore I believe the plugin should expose a custom error type (like EffectFailureError?) to throw with a non-error cause.

Let's make one assumption: If the plugin user had follow pothos's error plugin introduction then Error interface and BaseError object type should have been defined.

How about just throwing Error which set cause field? if plugin user wants to check original error, could be add new field in BaseError object.

if (Cause.isFailType(cause) && cause.error instanceof Error) {
  throw cause.error;
}

throw new Error(Cause.pretty(cause), { cause });

We can decide to expose a custom error type but, it should be defined as object type somewhere by user or the plugin itself.

from pothos-plugin-effect.

iamchanii avatar iamchanii commented on May 28, 2024

Update 2024: I recently worked on 1.0 of pothos-plugin-effect, and the API has changed in such a way that effectRuntime is now configured in SchemaBuilder. I expect that the initial cause of this issue will be resolved.

Thank you for the many discussions about the API in this issue. I wouldn't have been able to move forward without this discussion. If you get a chance, please take a look at my new work as well.

from pothos-plugin-effect.

Related Issues (17)

Recommend Projects

  • React photo React

    A declarative, efficient, and flexible JavaScript library for building user interfaces.

  • Vue.js photo Vue.js

    🖖 Vue.js is a progressive, incrementally-adoptable JavaScript framework for building UI on the web.

  • Typescript photo Typescript

    TypeScript is a superset of JavaScript that compiles to clean JavaScript output.

  • TensorFlow photo TensorFlow

    An Open Source Machine Learning Framework for Everyone

  • Django photo Django

    The Web framework for perfectionists with deadlines.

  • D3 photo D3

    Bring data to life with SVG, Canvas and HTML. 📊📈🎉

Recommend Topics

  • javascript

    JavaScript (JS) is a lightweight interpreted programming language with first-class functions.

  • web

    Some thing interesting about web. New door for the world.

  • server

    A server is a program made to process requests and deliver data to clients.

  • Machine learning

    Machine learning is a way of modeling and interpreting data that allows a piece of software to respond intelligently.

  • Game

    Some thing interesting about game, make everyone happy.

Recommend Org

  • Facebook photo Facebook

    We are working to build community through open source technology. NB: members must have two-factor auth.

  • Microsoft photo Microsoft

    Open source projects and samples from Microsoft.

  • Google photo Google

    Google ❤️ Open Source for everyone.

  • D3 photo D3

    Data-Driven Documents codes.