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

fastify-circuit-breaker caused fastify instance crash ? #109

Closed
2 tasks done
abcfy2 opened this issue Dec 21, 2022 · 4 comments
Closed
2 tasks done

fastify-circuit-breaker caused fastify instance crash ? #109

abcfy2 opened this issue Dec 21, 2022 · 4 comments

Comments

@abcfy2
Copy link

abcfy2 commented Dec 21, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the bug has not already been reported

Fastify version

4.4.0

Plugin version

3.1.0

Node.js version

16

Operating system

Linux

Operating system version (i.e. 20.04, 11.3, 10)

Manjaro 22

Description

I try to hook this plugin into a normal fastify project, but I got a crash issue.

Steps to Reproduce

Here is a sample to reproduce:

const setTimeout = require('timers/promises').setTimeout;
const fastify = require('fastify')();
const cb = require('@fastify/circuit-breaker');

fastify.register(cb)
  .after(err => {
    if (err) {
      console.trace(`register plugins failed: ${err.message}`);
      throw err;
    }
  })
  .register(router);

async function router(fastify) {
  fastify.register(controller, {prefix: '/'});
  fastify.addHook('preHandler', fastify.circuitBreaker());
}

async function controller(fastify) {
  fastify.get('/', async function (req, reply) {
    // make a long time request manually, and expect a timeout response
    await setTimeout(60000);
    reply.code(200).send({health: 'OK'});
  });

  // ... Omit other URL handling
}


fastify.listen({ port: 3000 }, err => {
  if (err) throw err
  console.log('Server listening at http://localhost:3000')
})

I have a normal action fastify.get('/'), and I wish this plugin could work when timeout.

node index.js

# open another terminal
curl -v localhost:3000

And curl seems working (but why I have to wait until 1 min, not default 10 s?):

$ curl -v localhost:3000
*   Trying 127.0.0.1:3000...
* Connected to localhost (127.0.0.1) port 3000 (#0)
> GET / HTTP/1.1
> Host: localhost:3000
> User-Agent: curl/7.86.0
> Accept: */*
> 
* Mark bundle as not supporting multiuse
< HTTP/1.1 503 Service Unavailable
< content-type: application/json; charset=utf-8
< content-length: 109
< Date: Wed, 21 Dec 2022 15:09:53 GMT
< Connection: keep-alive
< Keep-Alive: timeout=72
< 
* Connection #0 to host localhost left intact
{"statusCode":503,"code":"FST_ERR_CIRCUIT_BREAKER_TIMEOUT","error":"Service Unavailable","message":"Timeout"}

And node index.js crashed:

$ node index.js
Server listening at http://localhost:3000
node:events:491
      throw er; // Unhandled 'error' event
      ^

Error [ERR_STREAM_WRITE_AFTER_END]: write after end
    at new NodeError (node:internal/errors:393:5)
    at ServerResponse.end (node:_http_outgoing:968:15)
    at /tmp/tmp/node_modules/fastify/lib/error-handler.js:43:17
    at fallbackErrorHandler (/tmp/tmp/node_modules/fastify/lib/error-handler.js:130:3)
    at handleError (/tmp/tmp/node_modules/fastify/lib/error-handler.js:33:5)
    at onErrorHook (/tmp/tmp/node_modules/fastify/lib/reply.js:742:5)
    at wrapOnSendEnd (/tmp/tmp/node_modules/fastify/lib/reply.js:535:5)
    at handleReject (/tmp/tmp/node_modules/fastify/lib/hooks.js:238:5)
Emitted 'error' event on ServerResponse instance at:
    at emitErrorNt (node:_http_outgoing:827:9)
    at process.processTicksAndRejections (node:internal/process/task_queues:83:21) {
  code: 'ERR_STREAM_WRITE_AFTER_END'
}

Expected Behavior

I hope I can get timeout response when circuit-breaker trigger timeout event, and fastify instance should not crashed.

@abcfy2
Copy link
Author

abcfy2 commented Dec 21, 2022

It seems my feature request is #83

@Eomm
Copy link
Member

Eomm commented Dec 21, 2022

This should work.

Don't mix async/callback style

  fastify.get('/', async function (req, reply) {
    // make a long time request manually, and expect a timeout response
    await setTimeout(60000);
-    reply.code(200).send({health: 'OK'});
+   return {health: 'OK'}
  });

@abcfy2
Copy link
Author

abcfy2 commented Dec 21, 2022

Oh, thanks, this is working. But I also hope client will get timeout response imminently and not wait for the whole route handler finished.

Maybe I have to wait for #83 ?

@abcfy2
Copy link
Author

abcfy2 commented Dec 23, 2022

Close as #83 .

@abcfy2 abcfy2 closed this as completed Dec 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants