fix: `FILE_REFERENCE_EXPIRED` in ctx.DownloadMedia

TeaDove commented 2 months ago

Changes:

Swap SqliteSession to SqlSession in README.md (I guess the is no need for this change until beta17 release? If so, i'll remove is)
Add downloader example
Rename all receivers of ext.Context to ctx
Add ext.GetMediaFileName and ext.GetMediaFileNameWithId helpers.
Fix nil pointer dereference in fillUserIdFromMessage (I have no idea why, but without my fix master examples panics, this fix need's improvements)
Fix in DownloadMedia for Photo FILE_REFERENCE_EXPIRED (as in this issue)

TeaDove commented 2 months ago

I have found one very unpleasant bug. Even with this fix updates with type newChannelMessage don't have update.EffectiveUser() in such cases, where beta16 release works.

celestix commented 2 months ago

Hello @TeaDove! Thank you for this huge contribution, this really adds some important things that I missed out while writing the helpers. I have reviewed your changes, please make the requested changes so that we can proceed to merge.

TeaDove commented 2 months ago

Hi @celestix!

I don't see any requested changes

I'm not very good with github PR (used to gitlab), maybe i'm missing something? Or you meant you want me to fix fillUserIdFromMessage? (I can try, no problems) )

celestix commented 2 months ago

Hi @celestix!

I don't see any requested changes

I'm not very good with github PR (used to gitlab), maybe i'm missing something? Or you meant you want me to fix fillUserIdFromMessage? (I can try, no problems) )

Np, It's fine. Can you go to https://github.com/celestix/gotgproto/pull/62/files and check for my comments there (scroll through the code and you'll fine my comments in some places). Those are the requested changes.

TeaDove commented 2 months ago

@celestix sorry for late response

I have rechecked PR, but I haven't found any comments. If you have created multiple comments in one batch, maybe you haven't committed them? (pic related)

TeaDove commented 2 months ago

Because for me the is zero comments

celestix commented 2 months ago

Because for me the is zero comments

Screenshot_20240331_025623

Fine, I am posting a screenshot of review.

TeaDove commented 2 months ago

@celestix

I've moved all media functions to functions/mediaHelpers
I've reched issue with GetNewUpdates. With and without my change i'm getting nil pointer dereference on attempt to get user's username

client (@...) has been started... 2024/04/01 13:38:30 runtime error: invalid memory address or nil pointer dereference goroutine 84 [running]: runtime/debug.Stack() /opt/homebrew/opt/go/libexec/src/runtime/debug/stack.go:24 +0x5e github.com/celestix/gotgproto/dispatcher.(NativeDispatcher).handleUpdate.func1() /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:147 +0x9a panic({0x63d30e0?, 0x69340c0?}) /opt/homebrew/opt/go/libexec/src/runtime/panic.go:770 +0x132 main.download(0xc00015e320, 0xc000412550) /Users/teadove/projects/gotgproto/examples/downloader/main.go:63 +0x57 github.com/celestix/gotgproto/dispatcher/handlers.Message.CheckUpdate({0x64c2fb0?, 0xc0004fc150?, 0x0?, 0xc0?}, 0xc00015e320, 0xc000412550) /Users/teadove/projects/gotgproto/dispatcher/handlers/messages.go:40 +0xb1 github.com/celestix/gotgproto/dispatcher.(NativeDispatcher).handleUpdate(0xc00012e300, {0x64ca6a0, 0xc0001ee690}, {0x0, 0xc000414630, 0xc000414660, 0xc000414690}, {0x64ccea0, 0xc000424140}) /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:158 +0x2e4 github.com/celestix/gotgproto/dispatcher.(NativeDispatcher).dispatch(...) /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:137 github.com/celestix/gotgproto/dispatcher.(NativeDispatcher).Handle(0xc00012e300, {0x64ca6a0, 0xc0001ee690}, {0x64cd320?, 0xc00012eb40}) /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:128 +0x34c github.com/gotd/td/telegram.(Client).processUpdates(0xc0000dd008, {0x64cd320?, 0xc00012eb40}) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/handle_updates.go:27 +0x53c github.com/gotd/td/telegram.(Client).handleUpdates(0xc0000dd008, 0x5f9adf8?) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/handle_updates.go:53 +0x52 github.com/gotd/td/telegram.clientHandler.OnMessage({0x620237a?}, 0xe?) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/conn_builder.go:26 +0x13 github.com/gotd/td/telegram/internal/manager.(Conn).OnMessage(0x0?, 0x620237a?) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/internal/manager/conn.go:169 +0x22 github.com/gotd/td/mtproto.(Conn).handleMessage(0xc0002e2008, 0x660a8ea69ce68001, 0xc0004fe270) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/handle_message.go:41 +0x2cc github.com/gotd/td/mtproto.(Conn).handleGZIP(0xc0002e2008, 0x660a8ea69ce68001, 0x180299a6?) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/handle_gzip.go:23 +0x5d github.com/gotd/td/mtproto.(Conn).handleMessage(0xc0002e2008, 0x660a8ea69ce68001, 0xc0004fe210) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/handle_message.go:36 +0x195 github.com/gotd/td/mtproto.(Conn).consumeMessage(0xc0002e2008, {0x64ca6a0, 0xc0001ee0a0}, 0x0?) /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/read.go:83 +0x296 github.com/gotd/td/mtproto.(Conn).readLoop.func2() /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/read.go:207 +0x8f created by github.com/gotd/td/mtproto.(*Conn).readLoop in goroutine 70 /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/read.go:198 +0x2c7


I'm using my example with [downloader](https://github.com/celestix/gotgproto/pull/62/files#diff-61fd9143d65630a65390dac0d715efd45cdc4e02fccd0320e92a151de1524042R61):
```go
func download(ctx *ext.Context, update *ext.Update) error {
    // TODO remove after debug
    println(update.EffectiveUser().Username) // panic occures here

    filename, err := functions.GetMediaFileNameWithId(update.EffectiveMessage.Media)
    if err != nil {
        return errors.Wrap(err, "failed to get media file name")
    }
    ...

I'll delete println(update...) after fixes

celestix commented 1 month ago

@TeaDove 1.

2. https://github.com/celestix/gotgproto/blob/eb7062832de679cb523100da41c2bb06614ff074/ext/update.go#L114 Please make this

if u.userId == 0 {
        return nil
}

Please make these changes as well, I will merge this PR once the changes are done and you remove print statement from the example.

celestix commented 1 month ago

@TeaDove

@celestix

1. I've moved all media functions to functions/mediaHelpers

2. I've reched issue with GetNewUpdates.
   With **and** without my change i'm getting nil pointer dereference on attempt to get user's username

client (@...) has been started...
2024/04/01 13:38:30 runtime error: invalid memory address or nil pointer dereference
goroutine 84 [running]:
runtime/debug.Stack()
        /opt/homebrew/opt/go/libexec/src/runtime/debug/stack.go:24 +0x5e
github.com/celestix/gotgproto/dispatcher.(*NativeDispatcher).handleUpdate.func1()
        /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:147 +0x9a
panic({0x63d30e0?, 0x69340c0?})
        /opt/homebrew/opt/go/libexec/src/runtime/panic.go:770 +0x132
main.download(0xc00015e320, 0xc000412550)
        /Users/teadove/projects/gotgproto/examples/downloader/main.go:63 +0x57
github.com/celestix/gotgproto/dispatcher/handlers.Message.CheckUpdate({0x64c2fb0?, 0xc0004fc150?, 0x0?, 0xc0?}, 0xc00015e320, 0xc000412550)
        /Users/teadove/projects/gotgproto/dispatcher/handlers/messages.go:40 +0xb1
github.com/celestix/gotgproto/dispatcher.(*NativeDispatcher).handleUpdate(0xc00012e300, {0x64ca6a0, 0xc0001ee690}, {0x0, 0xc000414630, 0xc000414660, 0xc000414690}, {0x64ccea0, 0xc000424140})
        /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:158 +0x2e4
github.com/celestix/gotgproto/dispatcher.(*NativeDispatcher).dispatch(...)
        /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:137
github.com/celestix/gotgproto/dispatcher.(*NativeDispatcher).Handle(0xc00012e300, {0x64ca6a0, 0xc0001ee690}, {0x64cd320?, 0xc00012eb40})
        /Users/teadove/projects/gotgproto/dispatcher/dispatcher.go:128 +0x34c
github.com/gotd/td/telegram.(*Client).processUpdates(0xc0000dd008, {0x64cd320?, 0xc00012eb40})
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/handle_updates.go:27 +0x53c
github.com/gotd/td/telegram.(*Client).handleUpdates(0xc0000dd008, 0x5f9adf8?)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/handle_updates.go:53 +0x52
github.com/gotd/td/telegram.clientHandler.OnMessage({0x620237a?}, 0xe?)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/conn_builder.go:26 +0x13
github.com/gotd/td/telegram/internal/manager.(*Conn).OnMessage(0x0?, 0x620237a?)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/telegram/internal/manager/conn.go:169 +0x22
github.com/gotd/td/mtproto.(*Conn).handleMessage(0xc0002e2008, 0x660a8ea69ce68001, 0xc0004fe270)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/handle_message.go:41 +0x2cc
github.com/gotd/td/mtproto.(*Conn).handleGZIP(0xc0002e2008, 0x660a8ea69ce68001, 0x180299a6?)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/handle_gzip.go:23 +0x5d
github.com/gotd/td/mtproto.(*Conn).handleMessage(0xc0002e2008, 0x660a8ea69ce68001, 0xc0004fe210)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/handle_message.go:36 +0x195
github.com/gotd/td/mtproto.(*Conn).consumeMessage(0xc0002e2008, {0x64ca6a0, 0xc0001ee0a0}, 0x0?)
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/read.go:83 +0x296
github.com/gotd/td/mtproto.(*Conn).readLoop.func2()
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/read.go:207 +0x8f
created by github.com/gotd/td/mtproto.(*Conn).readLoop in goroutine 70
        /Users/teadove/go/pkg/mod/github.com/gotd/td@v0.99.1/mtproto/read.go:198 +0x2c7

I'm using my example with downloader:

func download(ctx *ext.Context, update *ext.Update) error {
  // TODO remove after debug
  println(update.EffectiveUser().Username) // panic occures here

  filename, err := functions.GetMediaFileNameWithId(update.EffectiveMessage.Media)
  if err != nil {
      return errors.Wrap(err, "failed to get media file name")
  }
  ...

I'll delete println(update...) after fixes

You're getting this panic on update.EffectiveUser() as it is returning a nil due to a wrongly evaluated condition which returns nil, it should be fixed once you make the following change:

TeaDove commented 1 month ago

@celestix it's working!

I have fixed everything you have said, update.EffenctiveUser().Username works fine! Seams like PR can be merged?

celestix commented 1 month ago

Hi @TeaDove, everything looks good to me now. Thanks for this important contribution!

celestix / gotgproto

fix: `FILE_REFERENCE_EXPIRED` in ctx.DownloadMedia #62