Transient and Request scopes do not work together

[aliu-bsm]

Bug Report

Current behavior

If the application has both transient and request scopes, the transient scoped service does not work. The services/controllers that use the transient service does not have the correct instance of the transient service

Input Code

Minimum reproducible repo

code to run.

$ npm run start
...
$ curl http://localhost:3000

---

• LoggerService is a transient service
• ContextService is a request service

The LoggerService is an empty object when a request comes in. Everything initialized the constructor is gone. Services/Controllers are not using the same instance that was created.

If either scope is removed, then both services work correctly according to its scope. It is only when there are transient and request scopes, then the transient scopes class won't work.

 

Some code snippets:

// logger.service.ts
@Injectable({
  scope: Scope.TRANSIENT,
})
export class LoggerService {
  prefix;
  logger;

  constructor() {
    this.logger = console;     // <--- ⚠️ It is called.
  }

  setPrefix(prefix: string) {
    this.prefix = prefix;
  }

  info(...args) {
    this.logger.log(`[${this.prefix}]`, ...args);
  }
}

// context.service.ts
@Injectable({
  scope: Scope.REQUEST,
})
export class ContextService {
  private context;

  setContext(obj: any): void {
    this.context = obj;
  }

  getContext(): any {
    return this.context;
  }
}

// app.controller.ts
@Controller()
export class AppController {
  constructor(
    private readonly log: LoggerService,
    private readonly context: ContextService,
    private readonly appService: AppService,
  ) {
    this.log.setPrefix('AppController');   // ⚠️ <-- `this.log.logger` is `undefined`
  }

  @Get()
  getHello(): string {
    this.context.setContext({
      time: new Date().toISOString(),
    });
    this.log.info('getHello');       // ⚠️ <-- `this.log.logger` is `undefined`
    return this.appService.getHello();
  }
}

// app.service.ts
@Injectable()
export class AppService {
  constructor(
    private readonly log: LoggerService,
    private readonly context: ContextService,
  ) {
    this.log.setPrefix('AppService');     // ⚠️ <-- `this.log.logger` is `undefined`
  }

  getHello(): string {
    this.log.info('getHello');      // ⚠️ <-- `this.log.logger` is `undefined`
    this.log.info(this.context.getContext());
    return 'Hello World!';
  }
}

Expected behavior

The LoggerService instances should have used the one that was created instead of an empty object while in the request scope.

Possible Solution

Environment

Nest version: 6.8.5 (latest at the moment)
 
For Tooling issues:
- Node version: `v10.16.3` and `v12.11.0` (both fails)
- Platform:  Mac

Others:
- NPM version: `6.9.0`

[kamilmysliwiec]

Fixed in 6.9.0

[ulrichb]

Hi @kamilmysliwiec. Since 6.9.0 the following request-scoped service doesn't work anymore.

@Injectable({ scope: Scope.REQUEST })
export class ApiAuthenticatedUserProvider implements UserProvider {

    constructor(@Inject(REQUEST) private readonly request: express.Request) {
        console.log('!!!!!', { request });
    }

    get user(): ApiUser {
        return xxxxxxx(this.request.user);
    }
}

=> request is null and the c'tor is called way too early and only once (it seems to be interpreted as SINGLETON).

Registration happens via { provide: name, useClass: clazz }.

When reverting to 6.8.5 everything works again.

[kamilmysliwiec]

@ulrichb if you use custom providers syntax (provide:) you should also define scope property: https://docs.nestjs.com/fundamentals/injection-scopes#usage (in this case, scope: Scope.REQUEST)

[ulrichb]

@kamilmysliwiec: Okay thanks, this fixes my problem. But why isn't it inferred from the decorator (as it seems to be in <= 6.8.5). Further it's a bit annoying to duplicate the scope information (also error prune when when doing refactorings).

[ulrichb]

Also note that I'm using useClass not useValue or useFactory (where it's clear that the scope must be specifed explicitly).

[kamilmysliwiec]

I'll push another fix shortly (to make it more straightforward :))

[ulrichb]

Okay cool. BTW: If not inferred from the decorator Nest should at least warn about a "mismatching" scope value when using useClazz custom providers and a scope-value is present in the decorator.

[ulrichb]

@kamilmysliwiec Did you already have a look at this? Or should I create a new issue for my "useClass scope vs. decorator scope" issue?

[lock]

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.