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

Add ability to inject/register request middleware #279

Open
LongLiveCHIEF opened this issue Mar 4, 2019 · 2 comments
Open

Add ability to inject/register request middleware #279

LongLiveCHIEF opened this issue Mar 4, 2019 · 2 comments
Labels
type:feature Changes add a new feature

Comments

@LongLiveCHIEF
Copy link

LongLiveCHIEF commented Mar 4, 2019

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 !#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 LongLiveCHIEF changed the title Add config option to BaseModel for logger middleware registration Add ability to inject/register request middleware Mar 4, 2019
@LongLiveCHIEF
Copy link
Author

LongLiveCHIEF commented Mar 4, 2019

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type:feature Changes add a new feature
Projects
None yet
Development

No branches or pull requests

2 participants