Improve error handling for missing Az.Accounts module
- Dominant language
- TypeScript
- Stars
- 552
- Forks
- 440
- Avg merge
- 3d 5h
- Merged PRs (30d)
- 18
Description
# Used version
`Download action repository 'azure/login@v2' (SHA:6c251865b4e6290e7b78be643ea2d005bc51f69a)` which points to [Release v2.1.1](https://github.com/Azure/login/releases/tag/v2.1.1).
# Issue description
In the workflow I have the following code:
```yaml
...
- 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:
https://github.com/Azure/login/blob/151fd0098c71af3d948d2ec8126743aa98926f5a/src/PowerShell/AzPSScriptBuilder.ts#L10-L11
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:
```diff
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`;
```
2. Mention somewhere in the README that `Az.Accounts` is required. I do see in [this issue's comment](https://github.com/Azure/login/issues/355#issuecomment-1867152849) 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.
Contributor guide
No contributing guide indexed for this repository
Assessment
This issue has not been assessed yet.