none
Risk of using a if in a try catch sentence of powershell RRS feed

  • Question

  • Hi everyone

    I'm currently working on a LDAP query to lookup every user of my company who are affected by some of my GPO and so my querry returns all of the user who are member of the group in my OU. My problem is sometimes I have groups who are member of the first group and then if I do a Get-ADUser of the group my powershell returns me an error and its normal. I have read on the forum that doing nested try was really a bad thing and then I though I could do a if in a try but was wandering if it could cause a problem? My script with the if and try together look something like this 

    function Group_Member_AD([string]$filePath,[string]$DNgroup){
       	$UserListLvl1= Get-ADGroupMember $DNgroup
    	foreach ($UserDN in $UserListLvl1){
    		Try{
    			if ($UserSurname = Get-ADUser $UserDN -Properties * -ea 0){
    				$DomainOwner="1"
    			}elseif($UserSurname = Get-ADUser -server subone.domain.com $UserDN -Properties * -ea 0){
    				$DomainOwner="2"
    			}elseif($UserSurname = Get-ADUser -server subtwo.domain.com $UserDN -Properties * -ea 0){
    				$DomainOwner="3"
    			}
    		}
    		Catch{
    			Group_Member_AD $filePath $UserDN
    		}
    I'm all ear to any thing I could improve in here
    Wednesday, January 15, 2014 3:16 PM

Answers

  • Personally, I wouldn't use try/catch for this purpose at all.  All AD objects have an "objectClass" property.  You could check this in your function instead of calling Get-AdUser and looking for an error:

    function Group_Member_AD([string]$filePath,[string]$DNgroup){
        $UserListLvl1= Get-ADGroupMember $DNgroup
        foreach ($UserDN in $UserListLvl1){
            if ($UserDN.objectClass -eq 'user')
            {
                # Do your User stuff here
            }
            elseif ($UserDN.objectClass -eq 'group')
            {
                Group_Member_AD $filePath $UserDN
            }
        }
    }

    You can still use try/catch to check for terminating errors, but there's no reason to rely on trapping an error from Get-ADUser to figure out that you should be treating $UserDN as a group instead.


    • Edited by David Wyatt Wednesday, January 15, 2014 3:58 PM
    • Proposed as answer by Jason Warren Wednesday, January 15, 2014 4:38 PM
    • Marked as answer by Francis Grégoire Wednesday, January 15, 2014 4:45 PM
    Wednesday, January 15, 2014 3:49 PM
  • Putting if statements inside of a try block is perfectly acceptable.

    Looking at this code, here is what I understand:

    You retrieve the membership of an Active Directory security group

    For each member of the group:

    - try to retrieve the user's object from AD

    - if you can't retrieve the object, try to retrieve it from a different server

    - if you can't retrieve the object, try to retrieve it from a second different server

    Is this correct? I'm not sure why you try Get-ADUser three times, please help me understand.


    Wednesday, January 15, 2014 3:48 PM

All replies

  • Putting if statements inside of a try block is perfectly acceptable.

    Looking at this code, here is what I understand:

    You retrieve the membership of an Active Directory security group

    For each member of the group:

    - try to retrieve the user's object from AD

    - if you can't retrieve the object, try to retrieve it from a different server

    - if you can't retrieve the object, try to retrieve it from a second different server

    Is this correct? I'm not sure why you try Get-ADUser three times, please help me understand.


    Wednesday, January 15, 2014 3:48 PM
  • Personally, I wouldn't use try/catch for this purpose at all.  All AD objects have an "objectClass" property.  You could check this in your function instead of calling Get-AdUser and looking for an error:

    function Group_Member_AD([string]$filePath,[string]$DNgroup){
        $UserListLvl1= Get-ADGroupMember $DNgroup
        foreach ($UserDN in $UserListLvl1){
            if ($UserDN.objectClass -eq 'user')
            {
                # Do your User stuff here
            }
            elseif ($UserDN.objectClass -eq 'group')
            {
                Group_Member_AD $filePath $UserDN
            }
        }
    }

    You can still use try/catch to check for terminating errors, but there's no reason to rely on trapping an error from Get-ADUser to figure out that you should be treating $UserDN as a group instead.


    • Edited by David Wyatt Wednesday, January 15, 2014 3:58 PM
    • Proposed as answer by Jason Warren Wednesday, January 15, 2014 4:38 PM
    • Marked as answer by Francis Grégoire Wednesday, January 15, 2014 4:45 PM
    Wednesday, January 15, 2014 3:49 PM
  • @jason warren

    I'm using the Get-ADUser three times because if I do a simple Get-ADUser without specifying the subdomain for some user powershell returns me an error that it cannot find the user. 

    @davud wyatt

    Thanks a lot, I'm not a really big fan of the try catch

    Wednesday, January 15, 2014 3:55 PM
  • David good call, checking the class of the object is a better approach than incurring the wrath of an exception.

    Wednesday, January 15, 2014 4:38 PM