softdevteam / mattermost-github-integration

GitHub integration for Mattermost
Other
79 stars 50 forks source link

Fix the application from error and outputting TooManyRedirects if there is a wrong ca… #9

Closed remimarenco closed 6 years ago

remimarenco commented 8 years ago

…se in the channel name (should be lowercase)

ptersilie commented 8 years ago

Thanks for the fix. Putting channel names to lower case is not enough, though. The channel names need to use the spelling of their URL, e.g. "Town Square" needs to be "town-square".

Maybe we shouldn't try to fix the spelling but instead just print a warning (possibly also return that warning to GitHub), telling people to check their channel names.

remimarenco commented 8 years ago

Hrmmm, is there a way to get from Mattermost the list of channels available and expose them in the warning?

So Error on not the good spelling, and list available channels in it to help people find the solution. The error on my side was quite leading not in the good path :/

ptersilie commented 8 years ago

I don't think you can easily query Mattermost for the available channels. I'd say the easiest solution is we catch the exception and print out a clue that it often occurs due to misspelled channel names:

try:
    r = requests.post(MATTERMOST_WEBHOOK_URL, headers=headers, data=json.dumps(data), verify=False)
    if r.status_code is not requests.codes.ok:
        print 'Encountered error posting to Mattermost URL %s, status=%d, response_body=%s' % (MATTERMOST_WEBHOOK_URL, r.status_code, r.json())
except requests.exceptions.TooManyRedirects:
    print "TooManyRedirects error. This issue is often caused by wrongly misspelled channel names (see Readme).'"
ptersilie commented 8 years ago

At least then we are doing a bit more than their own Gitlab plugin does which only prints out the trace: https://github.com/NotSqrt/mattermost-integration-gitlab/blob/master/mattermost_gitlab/server.py#L70