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

HTTP_SERVER_REF is undefined in TestingModule context #1282

Closed
BrunnerLivio opened this issue Nov 12, 2018 · 4 comments
Closed

HTTP_SERVER_REF is undefined in TestingModule context #1282

BrunnerLivio opened this issue Nov 12, 2018 · 4 comments

Comments

@BrunnerLivio
Copy link
Member

BrunnerLivio commented Nov 12, 2018

I'm submitting a...


[ ] Regression 
[x] Bug report
[ ] Feature request
[ ] Documentation issue or request
[ ] Support request => Please do not submit support request here, instead post your question on Stack Overflow.

Current behavior

I have a service, which injects the HTTP_SERVER_REF token from @nestjs/core and requests the HTTP server on application bootstrap

import {
  Injectable,
  Inject,
  OnApplicationBootstrap,
  HttpServer,
} from '@nestjs/common';
import { HTTP_SERVER_REF } from '@nestjs/core';
import { Server } from 'http';

@Injectable()
export class TerminusBootstrapService implements OnApplicationBootstrap {

  private httpServer: Server;

  constructor(
    @Inject(HTTP_SERVER_REF) private readonly httpAdapter: HttpServer,
  ) {}

  public onApplicationBootstrap() {
    this.httpServer = this.httpAdapter.getHttpServer();
  }
}

If I inject this service in a Testing Module Context (e2e) like so:

describe('AppController (e2e)', () => {
  let app: INestApplication;

  beforeAll(async () => {
    const moduleFixture = await Test.createTestingModule({
      // AppModule uses the `TerminusBootstrapService`
      imports: [AppModule],
    }).compile();

    app = moduleFixture.createNestApplication();
    await app.init();
  });

  it('/GET /', () => {
    return request(app.getHttpServer())
      .get('/health')
      .expect(200)
      .expect('Hello World!');
  });
});

I get the following error message:

this.httpAdapter.getHttpServer is not a function

Expected behavior

HTTP_SERVER_REF should provide a HTTP adapter in the Testing Module Context

What is the motivation / use case for changing the behavior?

Because of this error the e2e tests are failing of your app, if you use @nestjs/terminus TerminusModule in your application context.

Environment


Nest version: 5.4.0
 
For Tooling issues:
- Node version: 9.11.2
- Platform:  Linux

@BrunnerLivio
Copy link
Member Author

BrunnerLivio commented Nov 12, 2018

Injecting the HttpAdapter using moduleRef on application bootstrap does not work either:

...
public onApplicationBootstrap() {
    // Get the http adapter of nestjs
    this.httpAdapter = this.moduleRef.get<HttpServer>(HTTP_SERVER_REF); // Returns: {}
    this.httpServer = this.httpAdapter.getHttpServer();
  }

I think the problem lies in the e2e-spec file;

const moduleFixture = await Test.createTestingModule({
      // AppModule uses the `TerminusBootstrapService`
      imports: [AppModule],
    }).compile();

    app = moduleFixture.createNestApplication();
    await app.init();

The AppModule gets compiled first. During this compilation it sets the coreInjectables which HTTP_SERVER_REF is part of. The problem is, at this very moment the HttpAdapter is not set in the NestApplication. I assume this will get created in createNestApplication(). Afterwards the coreInjectables will not get updated, therefor the HTTP_SERVER_REF is null. Before I start any hacking and try to solve the problem on my own I want to hear what you, @kamilmysliwiec think how we should solve this.

@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Nov 19, 2018

Actually, HTTP_SERVER_REF is undefined because, in the testing context, HTTP server doesn't exist yet (which is an expected behavior). However, current status creates some difficulties with normal E2E testing, and therefore we'll switch to ApplicationReferenceHost soon (see @nestjs/core) - it should allow accessing HTTP_SERVER_REF lazily (you can already use it btw, just need to update docs)

@BrunnerLivio
Copy link
Member Author

Oh great, fixed it with nestjs/terminus#18 using the mentioned ApplicationReferenceHost.

nestjs/terminus is now ready from my side :)

nestjs/docs.nestjs.com#198

@lock
Copy link

lock bot commented Sep 24, 2019

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.

@lock lock bot locked as resolved and limited conversation to collaborators Sep 24, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants