jdalrymple / gitbeaker

🦊🧪 A comprehensive and typed Gitlab SDK for Node.js, Browsers, Deno and CLI
Other
1.55k stars 294 forks source link

Add ability to inject/register request middleware #279

Open LongLiveCHIEF opened 5 years ago

LongLiveCHIEF commented 5 years ago

Description Add ability to configure middleware for more control over requests and responses.

Proposal Implement a function on the BaseModel which would be a function type (or possibly an array of functions) that wraps each request/response and allows the implementor to pass custom request middleware. This would allow users to implement loggers such as Morgan or Winston, or allow them to integrate logs utilizing this api into existing application logs.

Implementation Details

This could be done in one of two ways,

  1. implementing a .use method on the BaseModel object
  2. implementing a middleware option on the BaseModelOptions interface.

I think both would achieve the ability for implementors to select what middleware they provide at either a global or service level, however the .use would probably be the better pattern, and it's more canonical with today's popular api's.

Basically, each request and response would be sent through applied middleware, allowing users to log/manipulate both incoming and outgoing traffic however they wish.

We could also take a look at environment-defined middleware, which would give users (and ourselves as developers) an easy hook for configuring middleware based on the globally defined environment option.

Related Issues

Possibly related to !https://github.com/jdalrymple/node-gitlab/issues/236. I decided this was a different feature, as it would provide a framework for building native logging/rotating options in the future.

Possible Benefits This could also open the door to natively supporting service-provided authentication mechanisms, making it easier to use this module in a distributed and scaling application.

This could also possibly allow users to add their own graphql server directly on top of the gitlab api's via this module. (still working this one out, but been giving some thought about how this module could either support or implement the graphql api).

LongLiveCHIEF commented 5 years ago

P.S. I'd like to be added as a collaborator. In addition to the idea above, I've been working on some documentation as i've gone about using this the last week or so. I may not stay a collaborator permanently, but I'm betting I'll be submitting a lot PR's in the next 2 months.

I was documenting how to pass query string options to the <Service>.all method, and I landed on the need for middleware support, as the results I'm getting back are not limited to the query string options I defined.

Also relevant, I'm working on updating some docs for gitlab's graphql api, and this work is tightly coupled. I've worked very closely with GitLab's doc and infrastructure teams over the last 4 years, and I'm the primary maintainer of the official puppet-gitlab config automation module.

jdalrymple commented 5 years ago

Ill close #236 As this provides a better way forward. The use pattern is definitely a clean way of moving forward. Something similar to:

import { Service } from 'gitlab'

const service = new Service(options)

service.use(some logger)

Or like you said including it in the service initialization options.

As for the docs, there has been some significant improvements in a #243 but i havent had the chance to rebase the branch and merge it. Id give that one a look to ensure youre not redoing work, might save you some time.