search

fleaz / CptHook

Receive webhooks from different applications and post them to IRC channels
MIT License
17 stars 8 forks source link

[GitLab] Use log.Error instead of log.Fatal for json parsing errors #37

Closed ManiacTwister closed 5 years ago

ManiacTwister commented 5 years ago

In the gitlabmodule log.Fatal is called when the module receives invalid/unexpected json data, which leads to program termination with exit status 1 - therefore it is possible to remotely "crash" CptHook. I think Log.Panic is more appropriate here?

fleaz commented 5 years ago

Hi, thanks for reporting this and and an even bigger thanks to already providing a PR :) The current behaviour is in fact not great.

According to the Logrus documentation Log.Panic calls the panic() function to gracefully shutdown stuff after logging the error. Wouldn't it be more sensible to just use Log.Error() in the cases where we receive malformed json?

ManiacTwister commented 5 years ago

Yeah makes sense :D Changed it to Log.Error

fleaz commented 5 years ago

Hi @ManiacTwister I fixed some issues with my branches and now this PR should be clean again if you rebase against development. Then I will be happy to merge it :)

ManiacTwister commented 5 years ago

@fleaz rebased, looks good now

fleaz commented 5 years ago

Awesome. Thanks :)