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

Create Policy type #1

Open
jakehamtexas opened this issue Sep 27, 2020 · 2 comments
Open

Create Policy type #1

jakehamtexas opened this issue Sep 27, 2020 · 2 comments
Assignees
Labels
hacktoberfest This issue is available for Hacktoberfest

Comments

@jakehamtexas
Copy link
Collaborator

jakehamtexas commented Sep 27, 2020

So looking over the original polly source code, it looks like there's lots of complex C#-specific features that go into its implementation, including extension methods on the PolicyBuilder type defined for each type of policy. This looks to be the main thing that drives the fluent api. We might be able to get really close to the fluent api that exists in the original, but for ease of contribution, it might be better to create distinct classes for each policy type and think about how to join them together/create a fluid api that encompasses all of them later on.

So my proposal for now is that we use an interface of some kind to unite them and start implementing them separately. Maybe something like the following:

type SyncOrAsyncFn = (() => Promise<void>) | (() => void));
interface Policy {
  execute: (fn: SyncOrAsyncFn) => Promise<void>
  // the original Polly has a very hard line between sync and async
  // usages of the `execute` function, and whoever its friends are in the
  // specific terminal policy that is built in a given usage, so we may likewise
  // want to keep sync and async fenced off. This interface has
  // them united into an async impl that handles sync cases.
}

Here's some pseudocode for RetryPolicy to continue the example:

class RetryPolicy<TException> implements Policy {
  or<T>(fn: ...) {
    // ...
    return this;
  }

  retry(numRetries: number) {
    // ...
    return this;
  }

   // etc.

  async execute(fn: SyncOrAsyncFn): Promise<void> {
    // ...
    await fn();
  }
}

Then we can create a module as our main export that at least gives us the impression of having a static Policy object for initializing policies. I'm hoping this can keep API changes a bit less disruptive but I'm not 100% sure how that might play out.

// index.ts
// this is the `main` file in the `package.json`

export default {
  handle: <TException>() => new RetryPolicy<TException>()
}

// elsewhere, in this library's calling code.

import Policy from 'pollyscript'

const retryPolicy = Policy.handle<MyCustomException>().or<WhateverException>().retry(3);

const fallibleFn = () => { ... };
retryPolicy.execute(fallibleFn);

NOTE: I'm completely spitballing the pseudo-implementation for retry policy. This version actually doesn't allow any meaningful usage outside of typescript, due to the use of generics as the driver for giving exception types in the main API, as an additional note.

@the-pat What do you think?

@the-pat
Copy link
Owner

the-pat commented Sep 27, 2020

It looks like our thinking is on similar tracks. Let's compare notes and decide how to move forward.

We might be able to get really close to the fluent api that exists in the original, but for ease of contribution, it might be better to create distinct classes for each policy type and think about how to join them together/create a fluid api that encompasses all of them later on.

I like how Polly uses the builder to generate the base policy, then tacks on the concrete policy at the end. I see what you're saying about ease of contribution. I'm still leaning towards starting with the fluent syntax and giving contributors a decent foundation to start with. What do you think?

const retryPolicy =
	Policy
		.handle(error => error implements CustomError)	// returns `PolicyBuilder`
		.or(error => error implements OtherCustomError)	// returns `PolicyBuilder`
		.retry(3);										// returns `RetryPolicy`

This version actually doesn't allow any meaningful usage outside of typescript, due to the use of generics as the driver for giving exception types in the main API, as an additional note.

I noticed the same thing. I think we'll have to pass functions like Error -> bool into handle and or to decide if the error is something that policy will handle. Thoughts?

@the-pat
Copy link
Owner

the-pat commented Sep 27, 2020

@jakehamtexas (due to the async nature that we get to experience with this project because of our relative global locations), I went through and added an untested version of the policy builder with a no-op policy as an example.

Let me know what you think! Here's the branch: https://github.com/the-pat/pollyscript/tree/policy-builder

@the-pat the-pat added hacktoberfest This issue is available for Hacktoberfest and removed hacktoberfest This issue is available for Hacktoberfest labels Sep 28, 2020
@the-pat the-pat added the hacktoberfest This issue is available for Hacktoberfest label Sep 28, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
hacktoberfest This issue is available for Hacktoberfest
Projects
None yet
Development

No branches or pull requests

2 participants