tomasfabian / Joker

Reactive data changes from SQL server to .NET clients. SqlTableDependency extensions, Joker.OData, Joker.Redis, Joker.MVVM and ksqlDB LINQ provider
MIT License
69 stars 23 forks source link

Error logging issue #5

Closed RajathVenkatesh closed 4 years ago

RajathVenkatesh commented 4 years ago

Hi

I had a situation recently where the SqlTableDependencyOnError method was executed but the OnError method which takes in an exception object was never executed when the application abruptly stopped.

This OnError method is important as I have an event source which I log into the windows event viewer for SCOM updates.

Thanks, Rajath

tomasfabian commented 4 years ago

Hi @Rajath, where exactly is your logging logic? Did you override SqlTableDependencyProvider.OnError? Can you reproduce the issue and provide me a call stack, please? I need additional information to solve this problem.

Regards, Tomas

RajathVenkatesh commented 4 years ago

@tomasfabian , Thanks for your quick response!

I do override the SqlTableDependencyProvider.OnError as well as SqlTableDependencyProvider.SqlTableDependencyOnError. But the problem is that only the SqlTableDependencyProvider.SqlTableDependencyOnError was executed while the SqlTableDependencyProvider.OnError which has the exception information was never executed, so hence I would not be able to provide you with a call stack.

Although I have a Event Log table which I use to log information when these two method get executed.

The ErrorEventArgs on the SqlTableDependencyProvider.SqlTableDependencyOnError method logged that TableDependency has stopped working which really is not much info to reproduce this.

Note : All other application related errors executes the SqlTableDependencyProvider.OnError and gets logged to my EventLog table with the stack trace.

A little background on my application -

I have a windows service which uses your extension to listen for table changes and perform inserts and updates on other tables in the database.

Now coming back to the issue :

This windows service is still running fine but the SqlTableDependencyProvider has stopped working and there are no trigger and service broker objects left in the database which is expected as the TableDependency has stopped working.

What would be your best advise when such errors occur ? to reinstantiate the SqlTableDependencyProvider in the SqlTableDependencyProvider.SqlTableDependencyOnError method ? It would really help me if you could let me know what is the flow cycle (the order in which error methods get called) when there is an error, it is sort of confusing as I have SqlTableDependencyProvider.SqlTableDependencyOnError as well as SqlTableDependencyProvider .OnError in your SqlTableDependencyProvider extension.

Rajath

tomasfabian commented 4 years ago

Hi Rajath, do you call base.SqlTableDependencyOnError in your override? It calls inside it OnError and tries to reconnect. Probably this will solve also your problem with reinstantiating.

    protected override void SqlTableDependencyOnError(object sender, ErrorEventArgs e)
    {
      base.SqlTableDependencyOnError(sender, e);
    }

I agree that it can be confusing. Should I hide SqlTableDependencyOnError (make it private)?

Tomas

RajathVenkatesh commented 4 years ago

@tomasfabian Okay this makes sense now I guess, I do not call the base constructor method (SqlTableDependencyOnError) inside my override which is the issue as you say.

I will add the block base.SqlTabelDependencyOnError(sender,e) inside the SqlTableDependencyProvider.SqlTableDependencyOnError override. I hope this will help me.

If SqlTableDependencyOnError override method has very limited logging and does not really provide an exception object I would prefer keeping it internal to your library implementation so I would suggest it to be private and not letting people override it (decision for you to make though). Just having OnError override as the single source for all error logging is what I would prefer.

I will give this a test in the coming few days and let you know.

Thanks again, Rajath

RajathVenkatesh commented 4 years ago

@tomasfabian Another question I have for you is :

do I need to call base.OnError(error) in the SqlTableDependencyProvider.OnError override method as well for reconnections?

Thank you as always.

tomasfabian commented 4 years ago

Hi, in version 2.1 you should also call base.OnError(error). I'm going to hide SqlTableDependencyOnError in the upcoming release and move the logic from base.OnError to a separate method, so it won't be mandatory to call it.

Tomas

tomasfabian commented 4 years ago

I released version 2.2.0.

Enjoy

RajathVenkatesh commented 4 years ago

After calling the base.OnError(error) in the SqlTableDependencyProvider.OnError override and calling base.SqlTabelDependencyOnError(sender, e) in the SqlTableDependencyProvider.SqlTableDependencyOnError in 2.1.0 version it all seems fine and the reconnection is working. If this might be helpful for someone in the future.

@tomasfabian in the latest version 2.2.0 do you want me only use the exception object in the SqlTableDependencyProvider.OnError override for my logging purpose and the reconnections will happen automatically and not make any base constructor calls? Please let me know so I can proceed updating to the newer version.

tomasfabian commented 4 years ago

@RajathVenkatesh, yes in version 2.2.0 you can override it like this and the reconnection will be done automatically:

    protected override void OnError(Exception exception)
    {
      logger.Log(exception);
    }

Thanks for the API improvement.

RajathVenkatesh commented 4 years ago

No, thank you.

I've given this a test and it's fine.

Rajath

tomasfabian commented 4 years ago

sorry @Rajath for my English. I wanted to write you: Thank you for your help with improving the API, it will hopefully help the other users too.

Tomas