feat: request module

[eddort]

Middlewares - is the main difference between the request module and the fetch module

You can add global middlewares into the Module instance. These middlewares will be used for each request.
For example, this standard middlewares will be repeated on each network error 5 times and change the base URL on each failure.

Let's set up our module

import { Module } from '@nestjs/common';
import {
  RequestModule,
  repeat,
  rotate,
  notOkError,
} from '@lido-nestjs/request';
import { ConfigModule, ConfigService } from './my.service';

@Module({
  imports: [
    ConfigModule,
    RequestModule.forRootAsync({
      async useFactory(configService: ConfigService) {
        return {
          middlewares: [repeat(5), rotate(configService.baseUrls), notOkError],
        };
      },
      inject: [ConfigService],
    }),
  ],
})
export class MyModule {}

And use it

import { RequestService } from '@lido-nestjs/request';

export class MyService {
  constructor(private requestService: RequestService) {}

  async myFetch() {
    return await this.requestService.json({ url: '/url' });
  }
}

[avsetsin]

I'm worried that we are generating dependencies in different packages. Can we reuse packages that are already in use in the lib?

[eddort]

You don't want to use this library?

[avsetsin]

could we use any instead?

[avsetsin]

Suggested change

	let response!: Response;
	logger.log('Start request', config);
	// eslint-disable-next-line prefer-const
	response = await next(config);
	logger.log('End request', config);
	return response;
	logger.log('Start request', config);
	const response = await next(config);
	logger.log('End request', config);
	return response;

[avsetsin]

we cannot store the entire url in the label:

• with a large number of unique urls the prom database will get huge
• the url may contain API keys

I think it's better to store only a domain name

[eddort]

Ok! I will fix it

[avsetsin]

Since you are not passing an error in args it will have success status in the metrics

in defaultSerializer:

result: error ? 'error' : 'success'

[avsetsin]

pls, don't use @ts-ignore

[avsetsin]

I think it's better to move this to packages/utils

[avsetsin]

why reduce and not map? did you copy deepClone from some package?

[avsetsin]

perhaps we should take the cloneDeep from lodash?

[eddort]

Yes, I do. I took it only for prototyping.

[avsetsin]

I guess that in service with a middleware design we should not have this configuration on a global level, but have it in the configs of the specific middleware responsible for repeating queries

[eddort]

What do you mean? Can I pass this variable to the context scope?

[avsetsin]

I think this module is written in more of a functional style and not very similar to the architecture of nestjs modules. It would be great if we could keep to one code style

[eddort]

I think a better way to create this module without some framework abstraction and wrote Nestjs binding. Because we don't need to use Nestjs abstraction inside this module. This way provides more clear code and it is a very popular way described in Uncle Bob's book.
And I think we should use this way in other modules in this repo. For example, middleware service looks very difficult but if we remove Nestjs abstraction from middleware code all ideas of that plugin will be looks like one line of code.

[avsetsin]

I don't think it's good that we're mutating the config object here. If a mutation is required, I would prefer to have a context arg here, which contains the config

[eddort]

Okay! I will change the config and rewrite this case and all the same cases