hedii / laravel-gelf-logger

A package to send gelf logs to a gelf compatible backend like graylog
MIT License
125 stars 33 forks source link

"Message is invalid" exception thrown by gelf-php when logging an empty string. #16

Closed Igneous closed 4 years ago

Igneous commented 4 years ago

Hi! First of all, thanks for making this wrapper. It's super useful. However, I've run into an interesting issue that I'd value your input on.

When logging an empty string with a call like Log::debug(''), the gelf-php library you're wrapping will throw this exception:

Message is invalid: short-message not set {"exception":"[object] (RuntimeException(code: 0): Message is invalid: short-message not set at /path/to/graylog2/gelf-php/src/Gelf/Publisher.php:71

We discovered this inside of a try/catch where a developer had left a call to Log::debug($ex->getMessage()). In this case, the exception message was an empty string. The gelf exception was actually being thrown inside of the catch block, and since there was nothing to catch that, we ended up serving 500s for this particular codepath for a little while :sweat_smile:.

I've corrected my original issue, and I think further problems are preventable on our side if I write a Processor. However, I thought it might be a good idea to bring this issue to your attention, since there's a difference between how any other monolog driver would handle an empty string vs laravel-gelf-logger.

If this isn't a bug, do you have any suggestions for how I could best prevent this in the future?

Thanks!

hedii commented 4 years ago

Hi. I'll check later, but i think the message field is mandatory on the gelf specs.

Meanwhile, you can write a custom processor, that check if message field is empty. If that's the case, replace it with a default message.

You can check https://github.com/hedii/laravel-gelf-logger/blob/master/src/Processors/NullStringProcessor.php as an example of how to write a processor.

hedii commented 4 years ago

In fact the message field is mandatory.

You can add a custom Processor, for example:

<?php

namespace App;

class EmptyMessageProcessor
{
    /**
     * Add a default message if message field is empty.
     *
     * @param array $record
     * @return array
     */
    public function __invoke(array $record): array
    {
        foreach ($record as $key => $value) {
            if ($key === 'message' && strlen($value) < 1) {
                $record['message'] = 'Your default message';
            }
        }

        return $record;
    }
}
delmicio commented 3 years ago

I did what hedii recommends.

<?php

namespace App\Logging\Gelf;

class AcceptEmptyMessageProcessor
{
    /**
     * Prevent error "Message is invalid: short-message not set"
     *
     * @param array $record
     * @return array
     */
    public function __invoke(array $record): array
    {
        if ($record['message'] === "") {
            $record['message'] = '<empty>';
        }

        return $record;
    }
}