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

Improve error handling for missing Az.Accounts module #463

Open
VincentVerweij opened this issue Jun 18, 2024 · 2 comments
Open

Improve error handling for missing Az.Accounts module #463

VincentVerweij opened this issue Jun 18, 2024 · 2 comments
Assignees
Labels
Azure PowerShell For Azure PowerShell issues. suggestion

Comments

@VincentVerweij
Copy link

Used version

Download action repository 'azure/login@v2' (SHA:6c251865b4e6290e7b78be643ea2d005bc51f69a) which points to Release v2.1.1.

Issue description

In the workflow I have the following code:

...

- name: Login to Azure
  uses: azure/login@v2
  with:
    tenant-id: ${{ secrets.AZURE_TENANT_ID }}
    subscription-id: ${{ secrets.AZURE_SUBSCRIPTION_ID }}
    client-id: ${{ secrets.AZURE_CLIENT_ID }}
    enable-AzPSSession: true

...

The error that I am getting is the following one:

{
  Error: "Cannot bind argument to parameter 'Name' because it is null.",
  Success: false
}
Error: Login failed with Error: Azure PowerShell login failed with error: Cannot bind argument to parameter 'Name' because it is null.. Double check if the 'auth-type' is correct. Refer to https://github.com/Azure/login#readme for more information.

I started investigating where this error message is coming from because "Cannot bind argument to parameter 'Name' because it is null." is not very descriptive.

Enabling the debug option gave me a pointer as to where this error message is originating:

$latestModulePath = (Get-Module -Name '${moduleName}' -ListAvailable | Sort-Object Version -Descending | Select-Object -First 1).Path
Import-Module -Name $latestModulePath

The debug output provided the following:

##[debug]            $latestModulePath = (Get-Module -Name 'Az.Accounts' -ListAvailable | Sort-Object Version -Descending | Select-Object -First 1).Path

If $latestModulePath is null, it will give the error above because Import-Module -Name $latestModulePath receives null.

Although I acknowledge that this is an issue with the self-hosted runner we're using, the error message could be improved.

To enhance this for future users of this action, we could improve it in two ways:

  1. If $latestModulePath is $null, it could show something like "Module 'Az.Accounts' not found, please install."
    One possible solution for AzPSScriptBuilder.ts could be:
let script = `try {
            $ErrorActionPreference = "Stop"
            $WarningPreference = "SilentlyContinue"
            $output = @{}
            $latestModulePath = (Get-Module -Name '${moduleName}' -ListAvailable | Sort-Object Version -Descending | Select-Object -First 1).Path
+           if ($latestModulePath -ne $null) {
              Import-Module -Name $latestModulePath
              $output['Success'] = $true
              $output['Result'] = $latestModulePath
+           }
+           else {
+             throw "Module '${moduleName}' not found, please install."
+           }
        }
        catch {
            $output['Success'] = $false
            $output['Error'] = $_.exception.Message
        }
        return ConvertTo-Json $output`;
  1. Mention somewhere in the README that Az.Accounts is required. I do see in this issue's comment that we need to install the Az.Accounts module if enable-AzPSSession is set to true. This could then also be referenced in the error message for example.

This improvement would give the user an idea of what is wrong, and what action they need to take to fix it.

@VincentVerweij VincentVerweij added the need-to-triage Requires investigation label Jun 18, 2024
@YanaXu YanaXu self-assigned this Jun 18, 2024
@YanaXu YanaXu added suggestion and removed need-to-triage Requires investigation labels Jun 18, 2024
@YanaXu
Copy link
Collaborator

YanaXu commented Jun 18, 2024

Hi @VincentVerweij, thanks for your advice. We'll add it to our plan.

@YanaXu YanaXu added the Azure PowerShell For Azure PowerShell issues. label Jun 18, 2024
@VincentVerweij
Copy link
Author

For whoever stumbles upon this in the future, I used this to fix the error about the missing module.

Just before calling Azure login, I added this step:

- name: Check and Install Az.Accounts module
  if: always()
  shell: pwsh
  run: |
    Write-Host 'Checking the Az.Accounts module...'
    try {
      Get-InstalledModule Az.Accounts -AllVersions -ErrorAction Stop
      Write-Host 'Az.Accounts module is already installed.'
    } catch {
      Write-Host 'Az.Accounts module is not installed. Trying to install...'
      Install-Module -Name Az.Accounts -Repository PSGallery -Force
    }
    Write-Host 'Done'

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

No branches or pull requests

2 participants