Consider some mechanism for handling worker errors in the default client?

[ccahoon]

Hello!

First of all, thanks for the great service and the great Go client.

I have not been able to find an existing mechanism in the library to be able to take action in the case of a worker failure in the default client, for example due to network issues (say, my server is unable to connect to Honeybadger's API).

I am able to work around it with your current implementation, but my solution feels a bit hackish. I am wrapping the Notify method of the default server backend so that I can do something when it fails (since errors that get returned to workers just get logged).

So, I have two possible proposals:

• Boolean "Synchronous" field in the Configuration struct
  • In the default Client's Notify method, check client.Config.Synchronous
  • If false: use the same behavior as now (which prevents me from using my own buffering and dealing with network failures, timeouts etc.)
  • If true: return the value of calling the backend's notify directly
  • Pros:
  • Backward compatible
  • Lets users implement their own queuing, circuit breakers, additional logic on failure & success
  • Cons:
  • User has to implement their own queuing/workers if they use it
  • This doesn't take the metric collector's use of asynchronous workers into consideration-- since they don't use the Client, and since their use of Notify is invisible to end-users, it might be confusing to have the Synchronous value impact error reporting in the metrics collector. I think this is a solvable problem, but I have not solved it (I am not currently using the metrics collector).
• Optional "WorkerErrors" channel on the client, which, if it is set, the worker writes to when it runs into issues with its work
  • Pros:
  • Backwards compatible
  • User doesn't have to implement their own workers
  • The metrics collector gets it for free
  • Cons:
  • User has less control over how the library talks to the API

I don't have a strong opinion—I didn't want to just write a feature request without trying to think it through. Either of these would solve the issue I'm having, and I think that it would be nice if the library provided access to worker errors in some fashion.

Thanks!

[joshuap]

Hey @ccahoon, thanks for the great writeup and I'm sorry for taking so long to get back to you! I think we can definitely implement one or both of these solutions. I'm doing some work this week to remove the metrics code (see our blog post), so that should simplify the first option. I'll think about this a bit more and then work on it after the metrics removal.

[ccahoon]

Great to hear! Congratulations on the code deletion—it's one of my favorite things.

I ran into a case last night where I got a lot of errors dropped (it was fine, they were all the same anyway) and I might have handled it better with one of these mechanisms. It feels nice at least to know that there's a concrete use case and that I'm not just asking you to do work for hypotheticals =).

[joshuap]

Update on this: (obviously) I haven't had time to work on it. I think the adding a Synchronous config option would be the way to go, though. I'm also open to suggestions of how to improve handling of failures in the worker (which I believe currently are just logged)--let's create new issues for those (now that the metrics collector is out of the picture).

If anyone wants to work on either of these two things, let me know. Otherwise I'll get to it eventually.

I'm going to close this issue and create a separate one for adding the Synchronous option. Thanks @ccahoon!