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

Fixes occasional crashes of Recurring Jobs page #730

Merged
merged 4 commits into from
Nov 16, 2016
Merged

Fixes occasional crashes of Recurring Jobs page #730

merged 4 commits into from
Nov 16, 2016

Conversation

aidmsu
Copy link
Contributor

@aidmsu aidmsu commented Nov 15, 2016

If there is problem with parsing of job cron expression Recurrig Jobs page will still be displayed.

This PR fixes #692

@@ -100,7 +100,7 @@
#if NETFULL
try
{
cronDescription = CronExpressionDescriptor.ExpressionDescriptor.GetDescription(job.Cron);
cronDescription = CronExpressionDescriptor.ExpressionDescriptor.GetDescription(job.Cron, new CronExpressionDescriptor.Options {ThrowExceptionOnParseError = false});
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please add spaces within curly braces:

{ ThrowExceptionOnParseError = false }

@odinserj
Copy link
Member

What CronExpressionDescriptor now returns in case of a null argument?

… the value of Resharper setting 'Witin single-line initializer braces' as true
@codecov-io
Copy link

codecov-io commented Nov 16, 2016

Current coverage is 67.34% (diff: 100%)

Merging #730 into master will decrease coverage by <.01%

Powered by Codecov. Last update ea3bc26...fdfa777

@aidmsu
Copy link
Contributor Author

aidmsu commented Nov 16, 2016

Now CronExpressionDescriptror returns 'Field 'ExpressionDescriptror.expression' not found'. I'll add one more commit after which CronExpressionDescriptor will throw exception in this case. Then the exception will be handled and a tooltip of cron expression field will be empty.

@odinserj
Copy link
Member

Thanks @aidmsu, 🚢 ping!

@odinserj odinserj merged commit 4207d29 into HangfireIO:master Nov 16, 2016
@odinserj odinserj added this to the 1.6.7 milestone Nov 16, 2016
@aidmsu aidmsu deleted the issue692 branch November 16, 2016 12:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Error on "Recurring Jobs" page
3 participants