nestjs / nest

A progressive Node.js framework for building efficient, scalable, and enterprise-grade server-side applications with TypeScript/JavaScript 🚀
https://nestjs.com
MIT License
67.7k stars 7.63k forks source link

Cannot catch ECONNREFUSED for microservice #288

Closed MartinLoeper closed 6 years ago

MartinLoeper commented 6 years ago

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.

Targeting an old issue which is not answered with information how it was solved: #127

Current behavior

I get the following error when I call this.client.send<number>(pattern, data); on a TcpClient when the service is down:

Error: connect ECONNREFUSED 127.0.0.1:8085
    at Object._errnoException (util.js:1026:11)
    at _exceptionWithHostPort (util.js:1049:20)
    at TCPConnectWrap.afterConnect [as oncomplete] (net.js:1174:14)

I cannot handle the error with a catch clause or a .catch on the Observable.

Expected behavior

I expect ECONNREFUSED to be catchable.

Minimal reproduction of the problem with instructions

@Component()
export class ProductionConnectorService {
    private log: Logger;

    @Client({ transport: Transport.TCP, port: 8085, host: "localhost" })
    private client: ClientProxy;

    constructor() {
        this.log = new Logger(ProductionConnectorService.name);
    }

    public call(): Observable<number> {
        const pattern = { cmd: 'sum' };
        const data = [1, 2, 3, 4, 5];

        try {
            return this.client.send<number>(pattern, data);
        } catch (e) {
            // Error not catched??!
            console.error(e);
        }
    }
}
@Get("/test")
 public async test() {
     let connection = this.prodConnectorService.call();

      return new Promise((resolve, reject) => {
         let promise = connection.toPromise();

         // set a timeout
         setTimeout(() => {
            reject();
         }, 7000);

         promise.then((data) => {
             resolve(data);
         }).catch((err) => {
             reject(err);
         });
     });
 }

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

I might catch the error and prevent node from crashing using the following code, but that is not the proper way:

process.on("uncaughtException", (err) => {
     console.error(err);
});

Environment


Nest version: 4.2.1

For Tooling issues:
- Node version: 8.5.0
- Platform:  Windows

Others: -
wbhob commented 6 years ago

I'd be happy to look into this with you. Can you share a repo with minimal reproduction that I can hack on a little?

MartinLoeper commented 6 years ago

Thank you for the kind offer!
I just created a test repository for you: https://github.com/MartinLoeper/nestjs-288.
However, by doing this I already found the bug (hopefully). The fix is proposed in https://github.com/nestjs/nest/pull/301.

kamilmysliwiec commented 6 years ago

Hi @MartinLoeper, Thanks for reporting 😃 It'll be fixed in the nearest update!

kamilmysliwiec commented 6 years ago

Hi @MartinLoeper, Please, update your packages into 4.5.0 and let me know whether it works or not 🙂

egbertc commented 5 years ago

I know this specific issue has been solved, but I'm seeing a very similar situation only backwards. I have 2 nestjs apps:

If the micro shuts down after connection is made the API won't crash. (the client calls will just fail until the micro comes back up... perfect!)

However, if the API shuts down, the micro crashes due to uncaught exception: 'read ECONNRESET'.

I'm not sure if I'm just missing a step in my micro setup or if this is a bug!

Here's a small repo that reproduces the issue: https://github.com/egbertc/nestjs-micro-crash-demo

lock[bot] commented 5 years ago

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.