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

README: throttle plugin API #1

Merged
merged 3 commits into from
Dec 18, 2018
Merged

README: throttle plugin API #1

merged 3 commits into from
Dec 18, 2018

Conversation

gr2m
Copy link
Contributor

@gr2m gr2m commented Dec 4, 2018

Here we can do some dreamcoding :) Curious how the API should ideally look like

Preview 👀

@zeke
Copy link

zeke commented Dec 4, 2018

Cool!

@gr2m gr2m changed the title README README: throttle plugin API Dec 5, 2018
@gr2m
Copy link
Contributor Author

gr2m commented Dec 18, 2018

ping @SGrondin 👀

@SGrondin
Copy link
Collaborator

SGrondin commented Dec 18, 2018

octokit.throttle.on('rate-limit', (retryAfter) => console.warn(`Rate-limit hit, retrying after ${retryAfter}s`))
octokit.throttle.on('abuse-limit', (retryAfter) => console.warn(`Abuse-limit hit, retrying after ${retryAfter}s`))

I like it! 👍

@gr2m
Copy link
Contributor Author

gr2m commented Dec 18, 2018

If you are happy with the API overall, I'd merge this in and we can start the implementation. I'm not very familiar with bottleneck so I hope you can help me out :)

One thing that concerns me a little about the octokit.throttle.on('abuse-limit', handler) API is that browsers don't have a native event emitter API and we'd need to bundle that into the browser builds as well. But I don't mind for now, we can change that later if necessary

@SGrondin
Copy link
Collaborator

I'll help as much as I can but I won't be able to write much code since my bandwidth is more limited than I thought. I'll review code and help as much as I can though! Bottleneck has a built in event emitter to get around the fact that browsers don't have one. All I have to do is expose it and then we could use it to add .on, .once, etc, to any object, or it could inspiration to write another one.

https://github.com/SGrondin/bottleneck/blob/master/lib/Events.js

@SGrondin
Copy link
Collaborator

Also, I'll add a custom build to Bottleneck just for this project, it'll be something like import Bottleneck from "bottleneck/browser";. It will be compiled to Node 10+ and will not contain any Clustering code. I'll release that this week. And if there's any features you need, I'll do my best to add them to Bottleneck within a few days. Of course, if you decide to go full custom and write your own custom throttling module I'm happy to help review it!

@gr2m
Copy link
Contributor Author

gr2m commented Dec 18, 2018

Thanks Simon!

@gr2m gr2m merged commit c939b56 into master Dec 18, 2018
@SGrondin
Copy link
Collaborator

SGrondin commented Dec 23, 2018

@gr2m As promised, I've just released Bottleneck 2.14.1, with the features I mentioned above:

Custom bundle

import Bottleneck from "bottleneck/light";
  • This bottleneck/light bundle is an UMD ES2017 file. I can probably just leave it as an ES Module (ESM format) as long as it's always imported by octokit and not directly by the browser. But UMD is just safer and will work with anything so I went with that.
  • Just like the standard Bottleneck module, it has 0 dependencies and runs in both Node and browsers.
  • It doesn't contain any Clustering code and trying to create a limiter with datastore: redis or datastore: ioredis will display an error message.
  • Its size is 38Kb and once minified it will be less than 20Kb.
  • I can also pre-minify it if you prefer (do you want sourcemaps? If so, embedded or not?)

Events micro-library

I've been using this for years to add events in a browser-compatible way without having to rely on a library. It lets you transform any object into an EventEmitter.

  • It adds .on(event, listener), .once(event, listener) and .removeAllListeners(event) to an object.
  • It handles exceptions inside of event listeners and emits them as 'error' events. It also handles failures inside 'error' event listeners themselves.
  • It supports returning a promise from an event listener as well as async event listeners.
  • It triggers a 'debug' event whenever a non-debug event is triggered. It's handy to help track down bugs in your code.

Here's a basic example using it inside of a class:

class HelloWorld {
  constructor() {
    this.emitter = new Bottleneck.Events(this);
  }

  doSomething() {
    this.emitter.trigger('info', 'hello', 'world', 123);
    return 5;
  }
}

const myObject = new Hello();

myObject.on('info', (...args) => {
  console.log(args); // ['hello', 'world', 123]
});

myObject.doSomething();

@gr2m
Copy link
Contributor Author

gr2m commented Dec 26, 2018

Thank you!

I can also pre-minify it if you prefer (do you want sourcemaps? If so, embedded or not?)

That’s not necessary

Events micro-library

Is this the code for the Events micro-library?
https://github.com/SGrondin/bottleneck/blob/master/src/Events.coffee

That looks small enough :)

It supports returning a promise from an event listener as well as async event listeners.

With that you mean it emits the special "error" event in event handlers even if they are async? Or is there more to it?

@gr2m gr2m deleted the initial-readme branch December 26, 2018 18:59
@SGrondin
Copy link
Collaborator

Is this the code for the Events micro-library?

Yup :)

With that you mean it emits the special "error" event in event handlers even if they are async? Or is there more to it?

  • If an event listener returns a promise (or an async function), the library will call the then() to let the chain continue.
  • If that then() returns a failed promise (or the async function throws an exception), the error is emitted as an 'error' event.
  • If a non-promise/non-async function throws an exception, the error is also emitted as an 'error' event.

Basically I never let errors disappear because that's a nightmare when trying to debug.

@gr2m gr2m mentioned this pull request Dec 27, 2018
3 tasks
@octokitbot
Copy link
Collaborator

🎉 This PR is included in version 1.0.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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.

4 participants