[BUG] Nested methods / Unhandled promise rejection

[gelito]

Hi all,

I'm trying to nest two router method. Like that.

  @All(':id*')
  checkCat(@Param('id', new ParseIntPipe()) id, @Req() Request) {
    Request.cat = {id, name: 'cat'};
    console.log('checkAll', Request.cat);
  }

  @Get(':id')
  findOne(@Param('id', new ParseIntPipe()) id, @Req() Request) {
    console.log('here', id, Request.cat);
    return Request.cat;
  }

But I found some issues.

First, If I return something on @All method, the process ends, so doesn't exec the @Get method. (Correct)

Second. If I don't return anything on @All method, the process doesn't end but also doesn't exec the @Get method (I don't think this will be a correct way)

Third. And most important. If I try to use the next() method

  @All(':id*')
  checkCat(@Param('id', new ParseIntPipe()) id, @Req() Request, @Next() next) {
    Request.cat = {id, name: 'cat'};
    console.log('checkAll', Request.cat);
    next();
  }

  @Get(':id')
  findOne(@Param('id', new ParseIntPipe()) id, @Req() Request) {
    console.log('here', id, Request.cat);
    return Request.cat;
  }

I obtain an Unhandled promise rejection because we're sent headers yet.

[ApplicationModule] Request...
[ApplicationModule] Request...
checkAll { id: 1234, name: 'cat' }
here 1234 { id: 1234, name: 'cat' }
(node:20321) UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): Error: Can't set headers after they are sent.
(node:20321) [DEP0018] DeprecationWarning: Unhandled promise rejections are deprecated. In the future, promise rejections that are not handled will terminate the Node.js process with a non-zero exit code.

[gelito]

Seen the code the problem stay on router-response-controller.ts:

    const res = response.status(statusCode);
    if (isNil(result)) {
      return res.send();
    }
    return isObject(result) ? res.json(result) : res.send(String(result));

You always set a status and send a response.

I think that an approach could be :

• Create a @hasNext() decorator
• Check if it exists and changes the router-response-controller.ts with something like that:

  public async apply(resultOrDeffered, response, requestMethod: RequestMethod, httpCode: number, hasNext: boolean = false) {
    const result = await this.transformToResult(resultOrDeffered);

    if (hasNext && isNil(result)) {
      return;
    }

    const statusCode = httpCode ? httpCode : this.getStatusByMethod(requestMethod);
    const res = response.status(statusCode);

    if (isNil(result)) {
      return res.send();
    }
    return isObject(result) ? res.json(result) : res.send(String(result));
  }

PS: I didn't see the full structure, so I'm sure there are better solutions.

[cojack]

@gelito your concept is strange as hell...

[gelito]

@cojack what is your doubt?? The nested components? Or the solution?? (I know that the solution doesn't look so well)

About "nested components": It's a basic Express feature: http://expressjs.com/en/guide/routing.html#route-handlers

[wbhob]

Try using a service. This is a really bizarre implementation, and there must be a better way.

[kamilmysliwiec]

Hi @gelito,
You're receiving UnhandledPromiseRejectionWarning: Unhandled promise rejection (rejection id: 2): Error: Can't set headers after they are sent. because you're not using @Res() decorator. It's a bug, Nest should not return anything when the @Next() decorator is used too. As a workaround, let's inject @Res() to the checkCat method for now.

[gelito]

Thx @kamilmysliwiec its works with @Res() decorator. I have to study better the project code to try to offer better solutions! ;-)

Anyway, thx.

@wbhob why you said it looks bizarre?? (if you are talking about the code, for sure! thanks that @kamilmysliwiec is much better than me! :-p )

[wbhob]

😂 glad it got fixed

[cojack]

@gelito nested components, it's weird...

[gelito]

@cojack imagine /books/:bookHash/chapters/:chapterHash

How do you do it? For me the more easy is the possibility of nested components. On books the @all method find it and add it to req. variable. And on the @get method of chapter you search for the chapter of the book.

Based on the idea of single responsibility, the Chapter component doesn't have to search if the bookHash is correct.

Better explained now? Any alternative to do it better or example? I'm always open to better ideas! 😉

[adrien2p]

Hey @gelito for me this is not the nested method which help, it’s just the route which are nested and each link to the appropriate controller.
If the route need to pass throught multiple controller, use policies or middleware to get the params and populate the context before to be arrived in tje controller method

[kamilmysliwiec]

@gelito same here, I'd rather use middlewares instead of passing the request through multiple controllers.

[adrien2p]

For example /books/:bookHash/chapters/:chapterHash should go on you chapterController method, before that it should pass through a middleware which will check the validity and populate the targeted book on the request to let you using it in your context.

[cojack]

@adrien2p more like https://symfony.com/doc/master/bundles/FOSRestBundle/6-automatic-route-generation_multiple-restful-controllers.html
@gelito check the link above

[kamilmysliwiec]

maybe possibility to declare parent inside the @Controller() decorator 😃

[gelito]

Okidoki, understood! I was trying to bring features from old structures...

I talk with the team your approach and we'll try to implement it to see the benefits ;-)

Thanks a lot.

[kamilmysliwiec]

A bug should disappear with 4.3.0 update 🙂

[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.