Pass HttpRequestMessage in ApiRequestEventArgs

[derigel23]

Hi!

I'm a little confused why so strange parameters are passed with ApiRequestEventArgs? Why not pass the whole HttpRequestMessage instead of parts of request like method and content like now? In consistence with ApiResponseEventArgs which uses HttpResponseMessage (that what I expect from that API). Also, making TelegramBotClient.MakeRequestAsync virtual is a good extension point too.

[karb0f0s]

@derigel23 we are making MakeRequestAsync virtual in the next release. could you please elaborate a use-case for HttpRequestMessage in ApiRequestEventArgs?

@tuscen is there a specific reason why we don't pass HttpRequestMessage instead of HttpRequestMessage.Content? Passing HttpRequestMessage/HttpResponseMessage to event handlers actually looks more consistent.

[derigel23]

Honestly, I don't remember already 🙈. I was experimenting with Polly wrapping http calls inside telegram client.

[derigel23]

@karb0f0s @tuscen I need a full HttpRequestMessage to pass loosely coupled data through HttpRequestMessage.Properties from IRequest to Polly policy (which have access only to raw HttpRequstMessage), for example, to pass target chat id to rate limit posting to it. Otherwise, with current implementation, I need to parse sending content to determine target chat ID.

[tuscen]

With the next version you'll be able to just override MakeRequestAsync method. I've made it virtual.

[derigel23]

It's still not enough. As an inheritor, I am still not able to get full original httprequest and I am not able to reproduce all base logic as many methods used there are private.

[tuscen]

What is your use case other than getting chat id from request?

If you only need chat id and user id with overriding MakeRequestAsync you'll be able to do this:

public override async Task<TResponse> MakeRequestAsync<TResponse>(
    IRequest<TResponse> request,
    CancellationToken cancellationToken)
{
    switch (request)
    {
        case IChatTargetable rq:
        {
            var chatId = rq.ChatId;
            // rate limit request here
            await rateLimiter.WaitAsync(chatId, cancellationToken);
        }
        case IUserTargetable rq:
        {
            var userId = rq.UserId
            // rate limit request here
            await rateLimiter.WaitAsync(userId, cancellationToken);
        }
    }

    return await base.MakeRequestAsync(request, cancellationToken);
}

[derigel23]

@tuscen Yes, something like that. But only as I've said I'm doing it with the standard RateLimit Polly policy. Which is configured externally and independently and bound to HttpClient. So, no knowledge about ITelegramClient. The only place to connect IRequest and rate limits is HttpRequestMessage. And, I need not only the whole message (instead of its content) in ApiRequestEventArgs but also the whole IRequest (instead of its method) in ApiRequestEventArgs too. The same symmetrically in ApiResponseEventArgs. Yes, it's a breaking change but that's for major version is. Also, I do not see any usages of events in the wild. And, if there are ones, they can get all the necessary info from new events.

[tuscen]

Done in #1102