slack-go / slack

Slack API in Go, originally by @nlopes; Maintainers needed, contact @parsley42
https://pkg.go.dev/github.com/slack-go/slack
BSD 2-Clause "Simplified" License
4.64k stars 1.13k forks source link

Large File upload causes timeout #1108

Closed DestroyerAlpha closed 1 year ago

DestroyerAlpha commented 2 years ago

What happened

I was experimenting different file size uploads to my slack workspace. The client timed out with a file size of about 50MB. The timeout happens after 20secs consistently.

Expected behavior

Send the file

Steps to reproduce

slackClient.UploadFile as well as slackClient.UploadFileContext with 1 minute context timeout, both fail with:

slack-go/slack2022/09/02 14:00:08 misc.go:272: HTTP/2.0 408 Request Timeout
Content-Length: 14
Content-Type: text/plain
Date: Fri, 02 Sep 2022 08:30:08 GMT
Server: envoy
Via: envoy-edge-bom-qrmq
X-Edge-Backend: envoy-www
X-Slack-Edge-Shared-Secret-Outcome: no-match

stream timeout

reproducible code

manifest.yaml

Versions

flowHater commented 1 year ago

It seems to be a serverside timeout. The default timeout for the http.Client{} is 0. which means no limit. The python lib for Slack got the same issue: https://github.com/slackapi/python-slack-sdk/issues/1165

As I can understand, they implemented an other function after exposing an other way in the Slack backend to upload File. They explained that here: https://github.com/slackapi/python-slack-sdk/releases/tag/v3.19.0

DestroyerAlpha commented 1 year ago

Any plans for similar support on slack-go

sarthak-kothari commented 1 year ago

Any plans on updating the API? Seeing similar issues

kanata2 commented 1 year ago

@flowHater Thanks for your investigation!

kanata2 commented 1 year ago

I'm reviewing @sarthak-kothari's PR (#1130) now.

kanata2 commented 1 year ago

Released v0.12.0 which includes #1130.

ghost commented 1 year ago

Hey @kanata2 no #1130 in https://github.com/slack-go/slack/releases/tag/v0.12.0

kanata2 commented 1 year ago

Oh...sorry.

flowHater commented 1 year ago

@kanata2 Can we expect a release soon with this patch ? Or are you waiting more commits to bundle the whole in a larger release ?

flowHater commented 1 year ago

What I saw, is that there is still pending comments on the MR, so It's not merged and so cannot be released.

DestroyerAlpha commented 1 year ago

@kanata2 Would it be possible to make UploadFileV2 analogous to UploadFile. The naming of the params struct as well as the expected parameters are different (e.g. UploadFile accepted a slice of channel IDs whereas UploadFileV2 only accepts a single channel as parameter.)