Ah ha. Okay, hit send and then saw this email. I see the logic now. Thanks, Geoff
From: listsadmin@lists.myitforum.com [mailto:listsadmin@lists.myitforum.com] On Behalf Of Michael B. Smith Sent: Tuesday, April 26, 2016 2:12 PM To: powersh...@lists.myitforum.com Subject: RE: [powershell] Try/Catch Format Question: Shoot. Made an error. Fixed in red below. From: listsadmin@lists.myitforum.com [mailto:listsadmin@lists.myitforum.com] On Behalf Of Michael B. Smith Sent: Tuesday, April 26, 2016 4:52 PM To: powersh...@lists.myitforum.com Subject: RE: [powershell] Try/Catch Format Question: Get-Verb shows you all the approved verbs in your current version of PS. So, and you can ask Web how much I harp on this, but I think error-reporting tends to be underdone in PowerShell. I prefer a rich set of mechanisms to protect yourself. IMHO, it is doing error detection, reporting, and recovery that takes a “useful” script and turns it into an enterprise-class script. So…. At the top of a script: $SaveEApref = $ErrorActionPreference $ErrorActionPreference = ‘Stop’ For a given cmdlet call: $error.Clear() [bool]$failed = $false [System.Exception]$e = $null [object]$result = $null try { $result = Get-AdUser John-Jacob-JingleHeimer-Schmitt $failed = !$? } catch { $failed = $true $e = $_ } if( $failed –or $error.Count –gt 0 ) { ## some error occurred Write-Error “Cmdlet failed: Get-AdUser” if( $null –ne $e ) { Write-Error “Exception: $( $e.ToString() )” } if( $error.Count –gt 0 ) { Write-Error “Last error: $( $error[ -1 ].ToString() )” } return 1 ## failure } ## now we can use $result … return 0 ## success ## at end of script $ErrorActionPreference = $SaveEAPref My $0.02. YMMV. Void in the State of Wisconsin. From: listsadmin@lists.myitforum.com<mailto:listsadmin@lists.myitforum.com> [mailto:listsadmin@lists.myitforum.com] On Behalf Of Devin Rich Sent: Tuesday, April 26, 2016 2:46 PM To: powersh...@lists.myitforum.com<mailto:powersh...@lists.myitforum.com> Subject: Re: [powershell] Try/Catch Format Question: I have 2 opinions on the matter and I can say that I am not an expert, so feel 100% free to completely disregard the rest of this email. First, Powershell generally operates on a Verb-Noun basis. When I read the function name "Enable-UserFailed" I sit here and wonder how I am going to 'enable' the 'UserFailed'. To me, the verb Enable doesn't describe what the function is doing and the noun 'UserFailed' doesn't describe what the verb is acting on. I like to look at this page for a bunch of available verbs when writing my function titles: https://msdn.microsoft.com/en-us/library/ms714428(v=vs.85).aspx May I possibly suggest Send or Out for your verb instead of enable (since these 2 functions seem to be related to reporting and not actually enabling something). I would also suggest changing your noun from UserSuccess\UserFailed to something like UserResult, SuccessfulCreation, etc. Thus you could have a function titled Send-FailedLyncUserCreation and I can surmise from that title that the function will alert to somewhere about how creating the user in Lync had a problem. Second, I use a try block as a conditional procedure block. If I want 2 commands to be attempted always, that I don't put them in the same try block (since 1 failing could cause the other to fail). I'll either use 2 try blocks, or maybe a try-catch-finally or just move the 2nd statement before the try. I have loved this post about try-catch: http://www.vexasoft.com/blogs/powershell/7255220-powershell-tutorial-try-catch-finally-and-error-handling-in-powershell I have not personally done an if (!$Error) before, but it seems like that could work if needed. I would suggest changing how the commands are ordered\working to avoid it though. I do think your code looks very good and clean. Keep it up! Don't forget to put in some documentation before you are done so next time you don't have to try and remember everything. :) Devin Rich Systems Administrator On Tue, Apr 26, 2016 at 12:00 PM, Orlebeck, Geoffrey <geoffrey.orleb...@chomp.org<mailto:geoffrey.orleb...@chomp.org>> wrote: All: I am working on a function for enabling users in Lync based off AD Group memberships. I am interested in the best practice when attempting to have try/catch perform an action only if there are no errors. I’ll include the entire code at the end of the email, but what I am curious about is this specific piece: If($Email -ne $Null) { try { Enable-CsUser $Member.Name -RegistrarPool $RegistrarPool -SipAddress "sip:$($Email)" -ErrorAction Stop Enable-UserSuccess } catch { Enable-UserFailed } } Else { Enable-UserFailed -Issue Email } Should the Enable-UserSuccess be called outside of the try block like this? If($Email -ne $Null) { try { Enable-CsUser $Member.Name -RegistrarPool $RegistrarPool -SipAddress "sip:$($Email)" -ErrorAction Stop } catch { Enable-UserFailed } If(!$Error) { Enable-UserSuccess } } Else { Enable-UserFailed -Issue Email } My logic for changing to using the If(!$Error) statement is in the first example if my event logging fails, then the entire try block fails, so the user would not get enabled in Lync. If the user account can be created, I want it created. The logging is there for myself/others, but is not critical to the success of the function. Anyway, here is the full code, in case it’s necessary. I apologize in advance for any formatting blunders. I’ve been reviewing the PS Styling documentation on GitHub (for those interested<https://github.com/PoshCode/PowerShellPracticeAndStyle/tree/master/Style%20Guide>) and am trying to adhere to it, but still not 100% cleaned up pre-existing scripts: Function Enable-LyncUsers { Param( # Group to parse members and enable in Lync [Parameter(Mandatory=$true, ValueFromPipeline=$true, ValueFromPipelineByPropertyName=$true, Position=0)] [ValidateNotNull()] [ValidateNotNullOrEmpty()] $CSGroup, # Registrar Pool to assign new Lync users [Parameter(Mandatory=$false, ValueFromPipeline=$true, ValueFromPipelineByPropertyName=$true, Position=1)] [ValidateNotNull()] [ValidateNotNullOrEmpty()] [string] $RegistrarPool = "AHPFILER.aspirehealthplan.org<http://AHPFILER.aspirehealthplan.org>" ) function Write-Log { [cmdletbinding()] Param( [Parameter(Mandatory=$true, Position=0)] [string] $Message ) $LogDirectory = "\\ahpmgmt\logs$\Lync\Enable_Users<file:///\\ahpmgmt\logs$\Lync\Enable_Users>" $LogFile = "$(Get-Date -UFormat "%Y.%m.%d") - Enable Lync Users.log" If(-not (Test-Path "$LogDirectory\$($LogFile)")) { New-Item -Path $LogDirectory -Name $LogFile -ItemType File } Add-Content -Path "$($LogDirectory)\$($LogFile)" -Value "$(Get-Date) | $Message$newLine" } function Enable-UserFailed { [cmdletbinding()] Param( [Parameter(Mandatory=$false, Position=0)] [string] $Source = "Enable Lync User", [Parameter(Mandatory=$false, Position=0)] [string] $Issue ) If(-Not [System.Diagnostics.EventLog]::SourceExists($Source)) { [System.Diagnostics.EventLog]::CreateEventSource($Source,'Lync Server') } If($Issue -eq "Email") { $Event = @{ LogName = "Lync Server" Source = $Source EventID = 8414 EntryType = "Warning" Message = "Failed to enable user, email not found: $($User.GivenName) $($User.Surname)" } Write-EventLog @Event } Else { $Event = @{ LogName = "Lync Server" Source = $Source EventID = 8414 EntryType = "Error" Message = "Failed to enable user: $($User.GivenName) $($User.Surname)" } Write-EventLog @Event } } function Enable-UserSuccess { [cmdletbinding()] Param( [Parameter(Mandatory=$false, Position=0)] [string] $Source = "Enable Lync User" ) If(-Not [System.Diagnostics.EventLog]::SourceExists($Source)) { [System.Diagnostics.EventLog]::CreateEventSource($Source,'Lync Server') } $Event = @{ LogName = "Lync Server" Source = $Source EventID = 8414 EntryType = "Information" Message = "User Enabled: $($User.GivenName) $($User.Surname)" } Write-EventLog @Event } Import-Module ActiveDirectory Import-Module Lync $Members = Get-ADGroupMember $CSGroup -Recursive Foreach($Member in $Members) { $Account = Get-CsUser -Identity $Member.SamAccountName -ErrorAction SilentlyContinue If($Account.Enabled -ne $True) { $User = Get-ADUser $Member.SamAccountName -Properties Mail $Email = $User.Mail If($Email -ne $Null) { try { Enable-CsUser $Member.Name -RegistrarPool $RegistrarPool -SipAddress "sip:$($Email)" -ErrorAction Stop Enable-UserSuccess Write-Log "Lync User Enabled: $($User.GivenName) $($User.Surname)" } catch { Enable-UserFailed Write-Log "Failed to enable user: $($User.GivenName) $($User.Surname)" } } Else { Enable-UserFailed -Issue Email Write-Log "No email found for user: '$($User.GivenName) $($User.Surname)'" } } } } Confidentiality Notice: This is a transmission from Community Hospital of the Monterey Peninsula. This message and any attached documents may be confidential and contain information protected by state and federal medical privacy statutes. They are intended only for the use of the addressee. If you are not the intended recipient, any disclosure, copying, or distribution of this information is strictly prohibited. If you received this transmission in error, please accept our apologies and notify the sender. Thank you. The information contained in this message is privileged, confidential, and protected from disclosure. If you are not the intended recipient, you are hereby notified that any review, printing, dissemination, distribution, copying or other use of this communication is strictly prohibited. If you have received this communication in error, please notify us immediately by replying to the message and deleting it from your computer. Confidentiality Notice: This is a transmission from Community Hospital of the Monterey Peninsula. This message and any attached documents may be confidential and contain information protected by state and federal medical privacy statutes. They are intended only for the use of the addressee. If you are not the intended recipient, any disclosure, copying, or distribution of this information is strictly prohibited. If you received this transmission in error, please accept our apologies and notify the sender. Thank you.
