Add a v2 client of hanu using the nlopes/slack client

[penguinpowernz]

This does not modify the existing client in any way, just adds a v2 package like many other go packages do. It uses the github.com/nlopes/slack package for the connections. #13 is also handled with this update.

[ChrisMcKee]

I like the idea of this; as I'm already mixing/matching between using hanu to handle the chat and nlopes to handle attachments / actions etc. I'm also hoping this handles disconnection better; as hanu in a long running proc seems to drop out (need to dig deeper on that one as nothing logs it just stops listening)

The readme changes are good but it still mentions using bot.listen() in v2; but you've changed the code to func (b *Bot) Listen(ctx context.Context) {

Good additions 👍

[ChrisMcKee]

also are you using the Bot User OAuth Access Token with this?
Seems to throw a not_allowed_token_type exception

[penguinpowernz]

Ah, nice catch with the Listen in the readme.

I was also having problems with the hanu websocket connection dropping out after a time, it's why I wanted to try the RTM connection from nlopes.

Sorry I didn't try to connect with it yet, will suck if we can't use the existing tokens :( More testing is required on my side for that.

[ChrisMcKee]

@penguinpowernz Im running the v1in a thread; hopefully it drops out when it cant do anything and it'll fall into a while loop; only recently started running it 24/7; new problems 😆

narrator: it didnt V1 stops listening but doesnt panic so the thread stays open while dead.
I cheated and added a slack command /resetbot as that hits a webapi and kills the pod.
Need to have a dig into the v1 issue; the nlopes stuff should work with bot tokens (as I use it with the bot token to push back already), the slack errors arent very verbose though / and it takes about 8 hours before it dies.

[ChrisMcKee]

//v2/bot_test.go
package hanu

import (
	"context"
	"log"
	"testing"
	"time"
)
var Start time.Time
func TestBot(t *testing.T) {
	bot, err := New("token")
	if err != nil {
		log.Fatal(err)
	}

	bot.Register(NewCommand("uptime",
		"Reply with the uptime",
		func(conv Convo) {
			conv.Reply("Thanks for asking! I'm running since `%s`", time.Since(Start))
		},))

	ctx, cancel := context.WithCancel(context.Background())

	defer cancel() // cancel when we are finished consuming integers

	bot.Listen(ctx)
}

outputs

2019/11/06 15:01:47 not_allowed_token_type
@ id, err := api.GetUserIdentity()

Thought I'd got passed that error; didn't click I'd written it against the v1 not v2 😆

[ChrisMcKee]

Changing the bot.go files new method to use auth test works fine. It doesn't like the GetUserIdentity
call

Because its not a user
Its a bot 🥂

func New(token string) (*Bot, error) {
	api := slack.New(token,
		slack.OptionDebug(true),
		slack.OptionLog(log.New(os.Stdout, "slack-bot: ", log.Lshortfile|log.LstdFlags)),
	)

	r, e  := api.AuthTest()
	if e != nil {
		return nil, e
	}

	rtm := api.NewRTM()
	bot := &Bot{RTM: rtm, ID: r.UserID}
	return bot, nil
}

Not sure about the context bit yet but the above at least passes through

---

update

In bot.go it was missing the manage connection call

func (b *Bot) Listen(ctx context.Context) {
	go b.RTM.ManageConnection()

Full file bot.go

I just need to marinate this for 16 hours 😆

Looking good though. I'm going to c+p the "New" so I have new with debug.

[ChrisMcKee]

With the above changes it's been running (using V2) just shy of 18 hours without issue 👍

It runs a keep alive so that'll help.

[ChrisMcKee]

Lost the user details on the conv though

The slack event comes back without the username; works fine though if you swap out name for id as thats what slacks using to chatter anyway

[penguinpowernz]

Oops, didn't see this chat before pushing latest, seems like I found the same info as you. Thank you for testing this! :)

I also added some realtime messaging while I was there, but totally changed the Message object to just wrap a nlopes/slack.MessageEvent object. It still satisfies the MessageInterface interface however, it just allows for some more context. Perhaps this will solve the 2nd error you found?

Edit: Oh my god, it's been 6 months!?

[penguinpowernz]

It's failing some tests though:

make gotest                                                                                                                                                                                 1 ↵
go test -v ./... -race
=== RUN   TestCommand
--- PASS: TestCommand (0.00s)
=== RUN   TestHandle
--- PASS: TestHandle (0.00s)
=== RUN   TestConversation
--- PASS: TestConversation (0.00s)
=== RUN   TestConnect
--- PASS: TestConnect (0.00s)
=== RUN   TestMessage
--- PASS: TestMessage (0.00s)
=== RUN   TestHelpMessage
--- PASS: TestHelpMessage (0.00s)
=== RUN   TestStripMention
--- PASS: TestStripMention (0.00s)
=== RUN   TestStripFormatting
--- PASS: TestStripFormatting (0.00s)
PASS
ok      github.com/penguinpowernz/hanu  1.018s
=== RUN   TestCommand
--- PASS: TestCommand (0.00s)
=== RUN   TestHandle
--- PASS: TestHandle (0.00s)
=== RUN   TestConversation
--- PASS: TestConversation (0.00s)
=== RUN   TestConnect
--- PASS: TestConnect (0.00s)
=== RUN   TestMessage
--- PASS: TestMessage (0.00s)
=== RUN   TestHelpMessage
--- PASS: TestHelpMessage (0.00s)
=== RUN   TestStripMention
--- FAIL: TestStripMention (0.00s)
    message_test.go:61: msg.Text must be 'help', is "<@test> help"
=== RUN   TestStripFormatting
--- FAIL: TestStripFormatting (0.00s)
    message_test.go:90: Failed to strip markup: 
         Got: <@test> how are you?
         Expected: how are you?
    message_test.go:90: Failed to strip markup: 
         Got: <@test> Hi <http://example.com|example.com> test <https://lorem.ipsum|ipsum.com> fail
         Expected: Hi example.com test ipsum.com fail
    message_test.go:90: Failed to strip markup: 
         Got: <@test> Hi <http://example.com|example.com> test <https://lorem.ipsum> fail
         Expected: Hi example.com test https://lorem.ipsum fail
    message_test.go:90: Failed to strip markup: 
         Got: <@test> Hi <slackbot://example@domain.tld>
         Expected: Hi slackbot://example@domain.tld
    message_test.go:90: Failed to strip markup: 
         Got: <@test> Hi <slackbot://example@domain.tld|label>
         Expected: Hi label
    message_test.go:90: Failed to strip markup: 
         Got: asdas <http://google.com> asdasd
         Expected: asdas http://google.com asdasd
FAIL
FAIL    github.com/penguinpowernz/hanu/v2       0.021s
FAIL
Makefile:4: recipe for target 'gotest' failed
make: *** [gotest] Error 1

I have had it working but not running for long periods of time.

[phanirithvij]

@penguinpowernz is there an issue with the function StripMention at https://github.com/penguinpowernz/hanu/blob/a5279beff036a663b1597e90274deff319a4e084/v2/message.go#L70 ?

Edit: I've fixed it and made a PR against your v2 branch penguinpowernz#1

[phanirithvij]

@penguinpowernz also would be great if you can add a go.mod

[ChrisMcKee]

@penguinpowernz I just forked in the end. Updated the slack refs etc. Mix and matched what I need from this with the core lib.
Slacks pushing it's event based / API style Comms route so it's nice to be able to pick and choose.

[penguinpowernz]

@phanirithvij sorry I missed that PR, I really need a better way to get my github notifications than email.

I added a go mod too, but the v1 and v2 schema is not supported now with go modules so its a bit awkward to import - I resorted to using the actually git commit SHA when doing the go get.

@ChrisMcKee and all sorry for being so slow to respond, I've really only must started using this thing in anger.

Slack better not remove the bot tokens, so much better to dial out than let something dial in, potentially exposing an attack vector.

I'm not finished with the changes yet and will likely merge my v2 folder into the root repo and chuck it in master, if anyone still cares to use the changes.

@sbstjn doesn't seem to be around so I'll just close this PR at that point.

Thanks

[penguinpowernz]

Closing this ticket, added the v2 code to master and tagged it with v2.0.0. If @sbstjn doesn't merge you can use my repo.