Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Seemingly unnecessary network request to api.nuget.org #4

Open
j-brooke opened this issue Mar 30, 2022 · 2 comments
Open

Seemingly unnecessary network request to api.nuget.org #4

j-brooke opened this issue Mar 30, 2022 · 2 comments

Comments

@j-brooke
Copy link

Based on the v1.1.0 code, it looks like Rest.Request is making a network call to api.nuget.org prior to every call to the SMSGlobal REST service. I believe this is unnecessary and wasteful.

The purpose seems to be obtaining the latest version of the package to put in the User-Agent header. It seems to me like it would be more valuable to include the currently running package version.

Suggestions to fix:

  1. If possible, eliminate the call to api.nuget.org, and use the current assembly's version. (For example, using typeof(Client).Assembly.GetName().Version.)
  2. If the Nuget call is necessary for some reason, call it once and cache the result in a singleton, or similar.
  3. If the Nuget call is necessary as part of every call, as it's coded now, add comments to explain why.

Also please note that the HttpClient instantiated on line 375 (as of tag 1.1.0) is not properly disposed.

@j-brooke
Copy link
Author

See pull request #5

@Place1
Copy link

Place1 commented Feb 29, 2024

We had an outage due to this hidden gem in the SDK. The nuget endpoint was returning an application/xml response for some reason (perhaps nuget had a disruption) which caused all SMS global requests to fail within the SDK.

System.Net.Http.UnsupportedMediaTypeException: No MediaTypeFormatter is available to read an object of type 'VersionsResponse' from content with media type 'application/xml'

The call to nuget is crazy and i'm left wondering how this implementation has been left in the SDK for so many years now or how it was allowed to exist in the first place...

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants