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

Report first-time install metric to Particular when a user installs NServiceBus #4280

Merged
merged 9 commits into from Nov 17, 2016
19 changes: 12 additions & 7 deletions packaging/nuget/tools/init.ps1
Expand Up @@ -33,14 +33,19 @@ try {

Set-ItemProperty -Path $platformKeyPath -Name "NuGetUser" -Value "true" | Out-Null


$url = "http://particular.net/download-the-particular-service-platform?version=$packageVersion"
$url = $url.ToLowerInvariant();

if($dte){
$dte.ExecuteCommand("View.URL", $url)
}
Write-Verbose 'Reporting first time install and version information to www.particular.net. This call does not collect any personal information. For more details, see the License Agreement and the Privacy Policy available here: http://particular.net/licenseagreement. Subsequent NuGet installs or updates will not invoke this call.' -verbose
$url = 'https://particular.net/api/ReportFirstTimeUsage'
$postData = New-Object System.Collections.Specialized.NameValueCollection
$postData.Add("version", $packageversion)
$wc = New-Object System.Net.WebClient
$wc.UseDefaultCredentials = $true
$wc.UploadValuesAsync($url,"post", $postdata)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we don't catch the exception here, what happens with the script if an exception is thrown? is that swallowed the same way as in a c# app or will this break the script?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@timbussmann - I have tested this scenario with a bad url. It does not break the script.
I also tested it by disabling the network adapter and running the nuget install (from local source), errors do not get reported.

}
Catch [Exception] {
Write-Warning $error[0]
}
finally {
if ($wc){
$wc.Dispose()
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this guaranteed to work? According to the doco upload async asynchronously uploads without blocking which means the finally block can be executed during the upload.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@danielmarbach - See http://stackoverflow.com/questions/1999858/how-bad-is-it-to-not-dispose-in-powershell

As far as I know, the call will execute asynchronously, even after the package manager is done and back to the console. I tested this by running the backend locally and set a breakpoint. The nuget install completed and got back to the prompt but the code was still invoked and waiting at the breakpoint I set.

Also this is a one time thing. We are not calling this script over and over. IMO, it shouldn't have any impact. What exact changes are you requesting here? Do you have any suggestions of what I can add here in the powershell?

@gbiellem - your thoughts on this?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think what @danielmarbach is getting at is since the powershell script is never awaiting the result of the async UploadValuesAsync call, that means the finally block could be executed and dispose the WebClient before the call finishes.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct what @bording said

}
}