honeybadger-io / honeybadger-php

PHP library for reporting errors to Honeybadger.io :elephant: :zap:
https://www.honeybadger.io/for/php/?utm_source=github&utm_medium=readme&utm_campaign=php&utm_content=website-url
MIT License
38 stars 21 forks source link

feat: events api integration #191

Closed subzero10 closed 5 months ago

subzero10 commented 5 months ago

Status

READY

Description

Honeybadger Insights integration. Adds a Honeybadger.event() function that allows sending events to Insights. Closes: #190

Todos

Next up

subzero10 commented 5 months ago
  1. There doesn't seem to be a timer to periodically flush the events queue, which could result in an event not being sent if it's added to the queue, but then another event isn't added, which could trigger the size or timeout thresholds being crossed.

PHP scripts are single threaded and usually short lived. For this reason, I opted for a shutdown listener to send any events that are still left in the queue when the script is exiting. I did come across a register_tick_function, which you can use to execute a callback every n-ticks (note: they don't translate to seconds). I didn't do it though because it may be unnecessary considering the above and I've never used it before. Additionally, different versions of PHP have different implementations of this feature. I'll ask TJ on Slack (thanks @joshuap) to get some feedback and can implement it in a follow-up PR if needed.

  1. The method signature for event doesn't support the option (as the Ruby gem does) to pass a hash instead of a string as the first argument.

I pushed a commit with an updated signature. I have to admit that I'm not loving this solution because it messes with PHP's latest improvements to type declarations. Unfortunately PHP doesn't have a good way for method overloading.

stympy commented 5 months ago

It looks like a001c91 requires event_type to be present in the payload that is sent to the API, but that shouldn't be the case. We don't need to require a value for event_type.

subzero10 commented 4 months ago

It looks like a001c91 requires event_type to be present in the payload that is sent to the API, but that shouldn't be the case. We don't need to require a value for event_type.

You are correct, nice catch! I just released v2.19.1 with a fix.