[Bug]: Modifying a message with attachments when hitting rate limits attempts to read/close/read the stream passed in

[jgoldshlag]

Check The Docs

• I double checked the docs and couldn't find any useful information.

Verify Issue Source

• I verified the issue was caused by Discord.Net.

Check your intents

• I double checked that I have the required intents.

Description

When calling ModifyMessageAsync and attempting to change/add attachments, if you are getting rate limited the stream you pass in will be read, sent to discord and then disposed when you get the rate limit error. It will then try to send it again, which will attempt to re-read the stream, which will fail for most streams. To work around this, I wrapped my stream in a wrapper that claimed it was not seekable, since the d.net code will copy a non-seekable stream to its own memoryStream and use (and dispose) that stream.

Version

3.3.2

Working Version

No response

Logs

System.ObjectDisposedException: Cannot access a closed Stream.
at System.IO.StreamHelpers.ValidateCopyToArgs(Stream source, Stream destination, Int32 bufferSize)
at System.IO.MemoryStream.CopyToAsync(Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
at Discord.Net.Rest.DefaultRestClient.SendAsync(String method, String endpoint, IReadOnlyDictionary2 multipartParams, CancellationToken cancelToken, Boolean headerOnly, String reason)
at Discord.Net.Queue.MultipartRestRequest.SendAsync()
at Discord.Net.Queue.RequestBucket.SendAsync(RestRequest request)
at Discord.Net.Queue.RequestQueue.SendAsync(RestRequest request)
at Discord.API.DiscordRestApiClient.SendInternalAsync(String method, String endpoint, RestRequest request)
at Discord.API.DiscordRestApiClient.SendMultipartAsync[TResponse](String method, String endpoint, IReadOnlyDictionary2 multipartArgs, BucketId bucketId, ClientBucketType clientBucket, RequestOptions options)
at Discord.API.DiscordRestApiClient.ModifyMessageAsync(UInt64 channelId, UInt64 messageId, UploadFileParams args, RequestOptions options)
at Discord.Rest.MessageHelper.ModifyAsync(UInt64 channelId, UInt64 msgId, BaseDiscordClient client, Action1 func, RequestOptions options)
at Discord.Rest.ChannelHelper.ModifyMessageAsync(IMessageChannel channel, UInt64 messageId, Action1 func, BaseDiscordClient client, RequestOptions options)
at Discord.WebSocket.SocketTextChannel.ModifyMessageAsync(UInt64 messageId, Action`1 func, RequestOptions options)

Sample

using MemoryStream memStr = new MemoryStream();
memStr.writeByte(123);
memStr.Position = 0;

await channel.ModifyMessageAsync(messageId, m =>
                    {
                        m.Content = "hi there";
                        m.Attachments = new List<FileAttachment>() { new FileAttachment(memStr, "zone_map.png")};
                    });

<.. repeat above ModifyMessageAsync a couple times to trigger a rate limit>

Packages

None

[quinchs]

Does this still occur in 3.4.1?

[jgoldshlag]

Yes it does, just hit this again

[quinchs]

I'm unable to reproduce this with this test code

                    try
                    {
                        var channel = client.GetGuild(848176216011046962).GetTextChannel(869966873368858708);

                        var msg = await channel.SendFileAsync(new FileAttachment(@"C:\Users\lynch\Downloads\video0_6.mov", "boop.mov", "lmao"), "haha");

                        bool sus = false;

                        while (true)
                        {
                            var att = sus ? new FileAttachment(@"C:\Users\lynch\Downloads\video0_6.mov", "boop.mov", "lmao") : new FileAttachment(@"C:\Users\lynch\Downloads\flipped.png", "lmao.png", "what") ;

                            Console.WriteLine("Starting to modify");
                            await msg.ModifyAsync(x => x.Attachments = new FileAttachment[] { att });
                            Console.WriteLine("Stoping modify");

                            sus = !sus;
                        }
                    }
                    catch(Exception x)
                    {
                        Console.WriteLine(x);
                    }

could you send some code over that hits this bug?

[jgoldshlag]

Hmm, I am also having trouble reproing it right now as well. I think I was seeing actual rate limits being triggered as well, and I am worried I wasn't actually running 3.4.1 when it happened again

[jgoldshlag]

So while I can't repro this in a test, it is definitely still happening with 3.4.1. I think it still has to do with not hitting pre-emptive rate limits, but getting rate limit responses from the discord server.

[ben-reilly]

I think I'm seeing this happen with 3.9.0; I'm copying messages from Slack over to a Discord server using SendFilesAsync, and sometimes when I get rate-limited the file stream gets closed and cannot be accessed.

I added some logging to my app and this is what it looks like when it happens (1565742619.0215 is the timestamp of the Slack message being processed; it acts as a message ID):

CLIENT (@1565742619.0215): Rate limit triggered: POST webhooks/1089635509061558333/6t60Nxp27m-LvjnoykSM30QWYXlxM4VX1WKAP9adjEKnEuEiQy2twbw_HFGT3xZ_9Yom?wait=true (Bucket: 3d2712a9e4fe17cc9d3fed4a8e672e5f)
Failed to send attachments from 1565742619.0215 (try 0)
CLIENT (@1565742619.0215): Rate limit triggered: POST webhooks/1089635509061558333/6t60Nxp27m-LvjnoykSM30QWYXlxM4VX1WKAP9adjEKnEuEiQy2twbw_HFGT3xZ_9Yom?wait=true (Bucket: 3d2712a9e4fe17cc9d3fed4a8e672e5f)
Failed to send attachments from 1565742619.0215 (try 1)
EXCEPTION WHILE SENDING MESSAGE AT TS 1565742619.0215
Cannot access a closed Stream.
   at System.IO.Strategies.BufferedFileStreamStrategy.CopyToAsync(Stream destination, Int32 bufferSize, CancellationToken cancellationToken)
   at System.IO.Stream.CopyToAsync(Stream destination)
   at Discord.Net.Rest.DefaultRestClient.SendAsync(String method, String endpoint, IReadOnlyDictionary`2 multipartParams, CancellationToken cancelToken, Boolean headerOnly, String reason, IEnumerable`1 requestHeaders)
   at Discord.Net.Queue.MultipartRestRequest.SendAsync()
   at Discord.Net.Queue.RequestBucket.SendAsync(RestRequest request)
   at Discord.Net.Queue.RequestQueue.SendAsync(RestRequest request)
   at Discord.API.DiscordRestApiClient.SendInternalAsync(String method, String endpoint, RestRequest request)
   at Discord.API.DiscordRestApiClient.SendMultipartAsync[TResponse](String method, String endpoint, IReadOnlyDictionary`2 multipartArgs, BucketId bucketId, ClientBucketType clientBucket, RequestOptions options)
   at Discord.API.DiscordRestApiClient.UploadWebhookFileAsync(UInt64 webhookId, UploadWebhookFileParams args, RequestOptions options, Nullable`1 threadId)
   at Discord.Webhook.WebhookClientHelper.SendFilesAsync(DiscordWebhookClient client, IEnumerable`1 attachments, String text, Boolean isTTS, IEnumerable`1 embeds, String username, String avatarUrl, AllowedMentions allowedMentions, MessageComponent components, RequestOptions options, MessageFlags flags, Nullable`1 threadId)
   at Tightrope.Models.Engine.SendMessage(DiscordMessage message, RestTextChannel botChannel, Dictionary`2 ghostGuilds, DiscordWebhookClient webhookClient) in C:\Users\ben\q\repos\tightrope\src\Tightrope\Models\Engine.cs:line 233
   at Tightrope.Models.Engine.PostMessagesToDiscord(Channel slackChannel, UInt64 channelId, ChannelMigrationProgress status) in C:\Users\ben\q\repos\tightrope\src\Tightrope\Models\Engine.cs:line 137

There's a simple retry loop in my code that immediately retries a failed send up to 3 times, each time re-creating the FileAttachment objects so that I have fresh streams. But that only saves me I think if the loop takes long enough that one of the attempts doesn't get rate-limited.

I'm sure that I could get around this by setting my loop up to wait until the rate limit timer has elapsed, which is what I'll try next, but I think it would be safest if the API did it for me.

[ben-reilly]

I've done some more investigating and I think this happens because request parameters are disposed of after the first attempt, regardless of whether it was a successful request. That disposal makes sense because all of the content gets dumped in a HttpRequestMessage in the following function, and that HttpRequestMessage needs to be disposed.

Discord.Net/src/Discord.Net.Rest/Net/DefaultRestClient.cs

Line 100 in 75ae488

public async Task<RestResponse> SendAsync(string method, string endpoint, IReadOnlyDictionary<string, object> multipartParams, CancellationToken cancelToken, bool headerOnly, string reason = null,

I think we both were seeing this with rate limiting because a 429 is one the few cases where a request is guaranteed to be re-sent with the exact same parameters. But we can actually replicate this bug with a much simpler case that reuses a param intentionally:

var webhookClient = new DiscordWebhookClient(...);
var fileAttachment = new FileAttachment(pathToFile);

// Reuse the same FileAttachment object for two messages
await webhookClient.SendFileAsync(fileAttachment, "Message 1");
await webhookClient.SendFileAsync(fileAttachment, "Message 2");

@quinchs I think your code sample was failing to repro because it was relying only on the 429 rate limit getting triggered correctly, and maybe it wasn't (see #2642). And you were creating a new FileAttachment on every loop, which circumvented the other repro I've just shared.

I think the solution here is to not provide the exact Stream object to the StreamContent / HttpRequestMessage object and instead making a copy every time. I'll create a PR to demonstrate and so we can discuss.