starred/monolog

Fork 0

mirror of https://github.com/Seldaek/monolog.git synced 2026-04-26 08:05:53 +03:00

[GH-ISSUE #289] HipChatHandler: Parameter validation/handling #96

New issue

Closed

opened 2026-03-04 02:12:08 +03:00 by kerem · 5 comments

kerem commented

2026-03-04 02:12:08 +03:00

Owner

Originally created by @levacic on GitHub (Dec 24, 2013).
Original GitHub issue: https://github.com/Seldaek/monolog/issues/289

The HipChat API (as per https://www.hipchat.com/docs/api/method/rooms/message) has a limit of 15 characters for the from parameter (the API docs say "less than 15", but 15 characters seem to be accepted).

When a HipChatHandler is instantiated with a longer name, perhaps it should throw an exception, or maybe only take the first 15 characters of the provided name. In my opinion, an exception is better, since the developer can instantly know they shouldn't do that, but maybe others have different preferences (I wasted some time figuring out why it wouldn't work, before I found out what happened, and an exception would have saved me that time).

Similarly, the message is limited to 10,000 characters, so it might be wise to automatically strip longer messages within HipChatHandler::buildContent() (it seems like an awfully bad idea to throw exceptions in this case, and stripping content shouldn't be a problem, as there is likely also a log file being written to). This change could have some performance implications, though, but I don't think they would be relevant (haven't benchmarked it, to be honest, it just makes sense).

I can implement these changes (or at least the first one, if people think the second one shouldn't be implemented) and send a PR, but I'd just like to know what others feel is the best approach to handle these cases.

Also, if an exception is to be thrown, what should its name be? I was thinking simply Monolog\Handler\HipChatHandlerException, so as not to complicate things too much (e.g. Monolog\Handler\HipChatHandler\LongNameException extending Monolog\Handler\HipChatHandler\Exception extending Exception, and similar bloat).

Originally created by @levacic on GitHub (Dec 24, 2013). Original GitHub issue: https://github.com/Seldaek/monolog/issues/289 The HipChat API (as per https://www.hipchat.com/docs/api/method/rooms/message) has a limit of 15 characters for the `from` parameter (the API docs say "less than 15", but 15 characters seem to be accepted). When a `HipChatHandler` is instantiated with a longer name, perhaps it should throw an exception, or maybe only take the first 15 characters of the provided name. In my opinion, an exception is better, since the developer can instantly know they shouldn't do that, but maybe others have different preferences (I wasted some time figuring out why it wouldn't work, before I found out what happened, and an exception would have saved me that time). Similarly, the message is limited to 10,000 characters, so it might be wise to automatically strip longer messages within `HipChatHandler::buildContent()` (it seems like an awfully bad idea to throw exceptions in this case, and stripping content shouldn't be a problem, as there is likely also a log file being written to). This change _could_ have some performance implications, though, but I don't think they would be relevant (haven't benchmarked it, to be honest, it just makes sense). I can implement these changes (or at least the first one, if people think the second one shouldn't be implemented) and send a PR, but I'd just like to know what others feel is the best approach to handle these cases. Also, if an exception is to be thrown, what should its name be? I was thinking simply `Monolog\Handler\HipChatHandlerException`, so as not to complicate things too much (e.g. `Monolog\Handler\HipChatHandler\LongNameException` extending `Monolog\Handler\HipChatHandler\Exception` extending `Exception`, and similar bloat).

kerem closed this issue

2026-03-04 02:12:08 +03:00

kerem commented

2026-03-04 02:12:09 +03:00

Author

Owner

@stof commented on GitHub (Dec 24, 2013):

Monolog is using the SPL exception. In this case, it should probably use InvalidArgumentException

@stof commented on GitHub (Dec 24, 2013): Monolog is using the SPL exception. In this case, it should probably use `InvalidArgumentException`

kerem commented

2026-03-04 02:12:09 +03:00

Author

Owner

@levacic commented on GitHub (Dec 24, 2013):

That's mostly true, but there's also the Monolog\Handler\MissingExtensionException, hence the question.

Anyway, InvalidArgumentException does seem like a good idea, so I'll wait a a few days to see if anyone else has any comments, and, if not, implement it like that.

Do you have any thoughts on how to handle the second case (message limit), or whether to handle it at all?

@levacic commented on GitHub (Dec 24, 2013): That's mostly true, but there's also the `Monolog\Handler\MissingExtensionException`, hence the question. Anyway, `InvalidArgumentException` does seem like a good idea, so I'll wait a a few days to see if anyone else has any comments, and, if not, implement it like that. Do you have any thoughts on how to handle the second case (message limit), or whether to handle it at all?

kerem commented

2026-03-04 02:12:09 +03:00

Author

Owner

@stof commented on GitHub (Dec 24, 2013):

The better way may be to split it into several messages. What do you think ?

@stof commented on GitHub (Dec 24, 2013): The better way may be to split it into several messages. What do you think ?

kerem commented

2026-03-04 02:12:09 +03:00

Author

Owner

@moderndeveloperllc commented on GitHub (Dec 24, 2013):

I would go with InvalidArgumentException. Perhaps a formatter would be appropriate as you are contemplating changing the content of the message. For truncating the message, look into mb_strimwidth. You would need to check if the function exists, but it should be fast enough not to have a big performance hit.

@moderndeveloperllc commented on GitHub (Dec 24, 2013): I would go with `InvalidArgumentException`. Perhaps a formatter would be appropriate as you are contemplating changing the content of the message. For truncating the message, look into `mb_strimwidth`. You would need to check if the function exists, but it should be fast enough not to have a big performance hit.

kerem commented

2026-03-04 02:12:09 +03:00

Author

Owner

@levacic commented on GitHub (Dec 24, 2013):

I've added a PR for the first part of the issue; the second seems like much more work, so it should probably have its own issue.

I'll tackle that as soon as I have more free time.

@levacic commented on GitHub (Dec 24, 2013): I've added a PR for the first part of the issue; the second seems like much more work, so it should probably have its own issue. I'll tackle that as soon as I have more free time.

kerem referenced this issue

2026-03-04 03:02:53 +03:00

[PR #96] [MERGED] Add CubeHandler #928