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

Uncaught exception inside cron crashes app #440

Closed
Boshen opened this issue Jan 18, 2021 · 4 comments
Closed

Uncaught exception inside cron crashes app #440

Boshen opened this issue Jan 18, 2021 · 4 comments

Comments

@Boshen
Copy link

Boshen commented Jan 18, 2021

I'm submitting a...


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

Current behavior

Uncaught exception inside cron crashes app
https://github.com/Boshen/nestjs-playground/blob/master/src/app.service.ts#L12-L15

  @Cron(CronExpression.EVERY_SECOND)
  handleCron() {
    console.log('triggerd cron')
    throw new Error('Error')
  }
triggerd cron
/Users/boshen/github/nestjs-playground/dist/main.js:1117
        throw new Error('Error');
        ^

Error: Error
    at AppService.handleCron (/Users/boshen/github/nestjs-playground/dist/main.js:1117:15)
    at CronJob.fireOnTick (/Users/boshen/github/nestjs-playground/node_modules/cron/lib/cron.js:562:23)
    at Timeout.callbackWrapper [as _onTimeout] (/Users/boshen/github/nestjs-playground/node_modules/cron/lib/cron.js:629:10)
    at listOnTimeout (node:internal/timers:556:17)
    at processTimers (node:internal/timers:499:7)

Expected behavior

Any of these:

  • Documentation hint - user should catch the error
  • nestjs wraps it and catch the error

I've dug through node-cron and it seems they don't have a global catcher also. But as a framework we should mitigate this somehow. My production app crashed a few times ;-)

@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Jan 18, 2021

Would you like to create a PR for this issue? :) A simple try..catch block with a logger.error() call should be sufficient

@Boshen
Copy link
Author

Boshen commented Jan 18, 2021

Sure, I'll give it a try

@kamilmysliwiec
Copy link
Member

Fixed in 0.4.2

@droplet-js
Copy link

droplet-js commented May 13, 2024

@kamilmysliwiec
Any news update? I have same issue.

  "dependencies": {
    "@nestjs/common": "^10.0.0",
    "@nestjs/core": "^10.0.0",
    "@nestjs/schedule": "^4.0.2",
  }
  @Timeout(10 * 1000)
  handleAfter10Seconds() {
    this.todo();
  }

  async todo() {
    await new Promise((resolve) => setTimeout(resolve, 10 * 1000));
    throw new Error('Error');
  }
    throw new Error('Error');
          ^
Error: Error

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

3 participants