search

tsg-ut / slackbot

TSGのSlackで動くSlackbotたち
MIT License
15 stars 12 forks source link

Legacy bot から脱却する #424

Closed hideo54 closed 2 years ago

hideo54 commented 3 years ago

今 TSG が使っている bot token は実は legacy.

単純に新しいやつ使いたいねというだけでなく、Slack SDK を使って綺麗に書ける部分が増えるし、今後どんどんサポートが切られていく可能性もあるので、ぜひとも移行しておきたい。 ただし、既存のテストを書き換える必要もあったりと、いささか面倒かも。そこのテストを諦めるという手もなくはないが。

ついでに HAKATASHI_TOKEN も USER_TOKEN とかわかりやすい名前に変えたい…変えたくない?

参考資料:

pizzacat83 commented 3 years ago

Legacy tokenを使っている理由はRTM APIが使えるというのが大きいと思っているのですが、最近Socket Modeという新生RTM API的なやつが出たのでそれを使うことで新しいtokenに移行できるかもしれません。(詳しく調べていないので、RTM APIの代用になるかはわかりません。)

cookie-s commented 3 years ago

なんかあとで精査が必要な文章

In our newer bot guide we do suggest that you use the Events API with bots in Slack apps; however you can also continue to use the Real Time Messaging API that was suggested for use with legacy bots.

pizzacat83 commented 3 years ago

お、マジ

pizzacat83 commented 3 years ago

New Slack apps may not use any Real Time Messaging API method. Create a classic app and use the V1 Oauth flow to use RTM. https://api.slack.com/rtm

まあ試してみるのが確実そうです

pizzacat83 commented 3 years ago

いや、よく考えると上は「今後作成するApp」に対する主張ですね

cookie-s commented 3 years ago

NewとClassicとLegacyな何かがあるのではという予想です（Slack、なんかドキュメントとか用語が一貫してるイメージはないので、こまる）

cookie-s commented 2 years ago

ちょっとこれやってみたいです。

想定手順です:

hakatashi commented 2 years ago

かなりよさそう。お願いします

cookie-s commented 2 years ago

まず、RTMが使える"Classic App"は https://api.slack.com/rtm#classic にあるリンク https://api.slack.com/apps?new_classic_app=1 から現在でも作成可能。

hideo54 commented 2 years ago

TSG の slackbot で RTM 使ってるのってほとんどメッセージ受信なので、雑に統合的に message event 受け取るサーバーを建てて (fastify plugin にするといいかな?)、そのへんまとめて移行したい気持ちあるなあ

cookie-s commented 2 years ago

俺はEvents APIはまだ考えてないぞ、Socket Modeができたらそれでとりあえず一回話を終わらせるからな

hideo54 commented 2 years ago

オイオイオイ Events API でええやん…よくない?

cookie-s commented 2 years ago

だってSocket Modeのほうが楽そうじゃない？少なくともぱっと見。

hideo54 commented 2 years ago

まあちょっとわかるんですが、一つは、Socket Mode ってまだそこまで成熟したものであるイメージがないんですよね (これは僕のキャッチアップ不足な気もする)。加えて…

移行コスト

Socket Mode is only available for apps using new, granular permissions. If you created your app on or after December of 2019, good news: your app already uses the new permissions. Otherwise, you may have to migrate your classic Slack app to use granular permissions before turning on Socket Mode. https://api.slack.com/apis/connections/socket

とありますが、

New Slack apps may not use any Real Time Messaging API method. Create a classic app and use the V1 Oauth flow to use RTM. https://api.slack.com/rtm

とあるとおり、RTM から完全離脱しないと Socket Mode は使えません。別 bot にするのも手間じゃないですか?

Node Slack SDK に含まれていない

Socket Mode を使うためには、今 Slack さんが激推ししてる Bolt を使うことになり、現 SDK と競合しうるので手間になります。 ……まあ、かたや Node Slack SDK は安泰かというと、

Deprecation Notice

@slack/events-api and @slack/interactive-messages officially reached EOL on May 31st, 2021. Development has fully stopped for these packages and all remaining open issues and pull requests have been closed. At this time, we recommend migrating to Bolt for JavaScript, a framework that offers all of the functionality available in those packages (and more). To help with that process, we've provided some migration samples for those looking to convert their existing apps. https://github.com/slackapi/node-slack-sdk#deprecation-notice

とあり、どちらにしろ Bolt への移行は避けられないのですが…

Events API と Socket Mode の違いはほとんどない

In Socket Mode, your app still uses the very same Events API and interactive components of the Slack platform. The only difference is the communication protocol. https://api.slack.com/apis/connections/socket#overview

「ほとんどないなら public HTTP endpoint を建てる必要がない Socket Mode の方がよいじゃん」という見方ももちろんあるりますが、上述の理由から、とりあえず Events API を使っておいて、RTM を駆逐できたら Socket Mode に移行する、という流れでいいんじゃないかと思います。その移行は (the very same を信用するなら) 比較的楽そう。

cookie-s commented 2 years ago

まず、Issueのタイトルは「Legacy botから脱却する」です。私はこれを、Classic Appでなくすることと捉えました。そしてその際に問題になるのがRTMですから、RTMを脱却することがほぼこのIssueのゴールと捉えています。

node-slack-sdkにsocket-modeは含まれています。単純にドキュメントの更新が追いついていないんだと思います。 https://www.npmjs.com/package/@slack/socket-mode

Classic botでなくなり、New Slack appとすることによって、最新のSlackのdocがまともに機能するようになります。たとえば、Slack docのpermissionなどは、New Slack app向けにかかれており、Classic Appのためには適宜読み替えたりしないといけないという認識です。

cookie-s commented 2 years ago

Events API と Socket Mode の違いはほとんどない

のであれば、(僕の中ではおそらく、くらいの確信度で)移行が容易なSocket Modeにしてから、Events APIに移行しよう、という手順で良いかと思います。 もちろん、Socket Modeが最終形として一番良いかはしらなくて、Events APIがSlackの推しであることは認識しています。勝手な予想ですが、Slackが、RTM勢がなかなか移行してくれないから用意したのがSocket Modeと思っていて、そうなのであればこれに乗るのがベターと考えています。まあ手順が長くなるんで、ぱっとEvents APIにしてくれる人がいればしてくれればいいんですが、このIssueは一年までではないですが放置されています。

hideo54 commented 2 years ago

oh, これはこれは…すいません。 > @slack/socket-mode

仰ることはすべて同意です。それがゴールです。その上で今、ゴールにたどり着くまでの間の経路のとり方について、主に実装コストの観点で話していました。段階的な移行ができる Events API の方が、すべて RTM を駆逐し終えてからでないと始められない Socket Mode よりも楽に作業できるのではという話です。

このIssueは一年までではないですが放置されています

これは本当にそのとおりで、結局じゃあ hideo54 はやるんかいとなると多分やらないので、最終的には作業のやる気のある人の判断でよいと思います

cookie-s commented 2 years ago

なるほどね　かなりEvents APIとかSocket Modeとかnode-slackのことがわかってきて、Socket Modeじゃなくていい気がしてきました　ありがとうございます もうちょっとがんばります

cookie-s commented 2 years ago

え、これもしかしてめちゃくちゃやるだけ？

hideo54 commented 2 years ago

プロ

cookie-s commented 2 years ago

まずは簡単 (rtm.on('message',くらいしかなさそう)なものから。 テストってえらいな～

cookie-s commented 2 years ago

ちなみにcommit push script

``` #!/bin/sh dirs="ahokusa \ anime \ atcoder \ better-custom-response \ checkin \ context-free \ dajare \ emoji-modifier \ emoxpand \ golfbot \ hakatashi-visor \ hangman \ hayaoshi \ kirafan \ lyrics \ mahjong \ octas \ pocky \ ponpe \ pwnyaa \ room-gacha \ sorting-riddles \ summary \ sunrise \ taiko \ tiobot \ vocabwar \ voiperrobot \ wordhero \ " for dir in $dirs do git checkout master git checkout -b $dir-deprecate-rtm git add $dir git commit -m "$dir: deprecate rtm" git push origin $dir-deprecate-rtm done ```
hideo54 commented 2 years ago

大変お疲れさまです。それぞれ数が多い中ご対応くださりありがとうございます。 Draft 以外の (Open or Merged の) 関連する Pull Request にすべて目を通し、レビューしました。

Events API, 割とこれまで通りに見えて、ちょっと来るデータ構造変わってたりします。レビューにおいて、thread_ts と username の2点が気になったので、それら2点が使われているものについては、次の通りコメントして request changes しました。

実験してないのでもし実際に来てたら申し訳ないのですが、ドキュメントを読んだ限りでは、`message.thread_ts` は来ない気がしています (`message.message.thread_ts` または `message.message.ts` になる気がする)。間違ってたらすいません。実験しましょう。
https://api.slack.com/events/message/message_replied
実験してないのでもし実際に来てたら申し訳ないのですが、ドキュメントを読んだ限りでは、`message.username` は来ない気がしています。間違ってたらすいません。実験しましょう。
https://api.slack.com/events/message

その他はそのまま引き継げると思いますので、accept しました。

横からしゃしゃり出て申し訳ないのですが一助になればと。これもし問題なかったら申し訳ねえな。よろしくお願いします。

hideo54 commented 2 years ago

特に thread_ts については、undefined になってしまっていたら即機能不全になってしまう気がします。これが使われていてかつ既に merge されてしまっているものは、#594, #599 です。ちゃんと動いているか確認してみたいですね。(僕は麻雀も pwnyaa もわからないので、誰かに任せます。)

hideo54 commented 2 years ago

ちなみに、完全に手前味噌ですが、channel notifier については、僕の管理している Slack では既に Events API 化していますので、ぜひご参照ください。結構構造変わっています。 https://github.com/hideo54/psi-slack/blob/main/channel-notifier.ts

お前が PR 出せやという声が聞こえて来そうだな。すいません。暇なときやります。

cookie-s commented 2 years ago 
eventClient.on('message', (message) => {
    console.log({message})
});
{
  message: {
    client_msg_id: '8c2a7e2b-b900-4fad-8a02-xxxxxxxxxxxxxx',
    type: 'message',
    text: 'ほげ',
    user: 'Uxxxxxxx',
    ts: '1641265473.000300',
    team: 'Txxxxxxxx',
    blocks: [ [Object] ],
    thread_ts: '1641265458.000200',
    parent_user_id: 'Uxxxxxxxx',
    channel: 'Cxxxxxxxx',
    event_ts: '1641265473.000300',
    channel_type: 'channel'
  }
}

閉廷！ｗ

cookie-s commented 2 years ago

usernameはしらん

cookie-s commented 2 years ago

https://github.com/slackapi/node-slack-sdk/blob/main/packages/web-api/src/response/ConversationsHistoryResponse.ts#L53 このへんですかね

cookie-s commented 2 years ago 
{
  message: {
    type: 'message',
    subtype: 'bot_message',
    text: 'ｼｭｯｼｭｯ (起動音)',
    ts: '1641266332.000700',
    username: 'tsgbot [DESKTOP-UXXXXX]',
    bot_id: 'Bxxxxxxxx',
    channel: 'Cxxxxxxxx',
    event_ts: '1641266332.000700',
    channel_type: 'channel'
  }
}

usernameもきたわ

cookie-s commented 2 years ago

https://github.com/slackapi/java-slack-sdk/blob/master/json-logs/samples/api/channels.history.json こういうのもあるらしいです

hideo54 commented 2 years ago

このドキュメントホンマ……

kurgm commented 2 years ago

username は人間の投稿にはつかなくて、bot の投稿の中でも chat.postMessage するときに username icon_emoji icon_url のうち 1 つ以上を設定した投稿にだけつくみたいですね

ちなみに、さっき実験してるときに分かったこととして、 username icon_emoji icon_url を設定して chat.postMessage するためには (classic apps でない場合) chat:write.customize とかいうスコープが追加で必要になるっぽいので、雑な例だったらついてなくても、まあ、みたいなところはあるかもしれない（？） このへん参照: https://api.slack.com/methods/chat.postMessage#authorship

hideo54 commented 2 years ago

なるほどなあ 実験ありがとうございます :bow: またしてもやってしまったきつねさん (ごめん…)

cookie-s commented 2 years ago

かんたんなのは終わって、続きをちょっと気をつけてやっていくよ

cookie-s commented 2 years ago

残り