Repository navigation
fix(graph): a group is judged by its member counts, and a throttled request is sent again - #19
Conversation
fadwen
left a comment
There was a problem hiding this comment.
Notes on the lines whose reason the diff does not show.
| } | ||
| $response = if ($exception.PSObject.Properties['Response']) { $exception.Response } else { $null } | ||
| if (-not $status -and $response) { $status = [int]$response.StatusCode } | ||
| if ($status -notin 429, 503, 504) { return $null } |
There was a problem hiding this comment.
502 is left out on purpose: Graph documents 429, 503 and 504 as the codes to retry, and a 502 from the gateway has not been seen to clear on a retry. 401 and 403 are never retried; a new token is Connect-MgGraph's job.
| $delay = [Math]::Ceiling(($retryAfter.Date.UtcDateTime - [datetime]::UtcNow).TotalSeconds) | ||
| } | ||
| } | ||
| if ($delay -lt 1) { $delay = [Math]::Pow(2, $Attempt) } |
There was a problem hiding this comment.
A Retry-After of 0, or a date already past, falls back to the exponential wait rather than retrying at once, so a server that says "now" still gets a pause.
| } | ||
| } | ||
| if ($delay -lt 1) { $delay = [Math]::Pow(2, $Attempt) } | ||
| [int][Math]::Min($delay, 60) |
There was a problem hiding this comment.
Capped at a minute: Graph has been seen to send Retry-After values of several minutes for a tenant-wide throttle, and a pre-flight that sits that long should fail and say so rather than appear hung.
| $total = [int](Invoke-IslGraphRequest -Uri "/v1.0/groups/$GroupId/members/`$count" @countSplat) | ||
| $deviceUri = "/v1.0/groups/$GroupId/members/microsoft.graph.device/`$count" | ||
| $devices = [int](Invoke-IslGraphRequest -Uri $deviceUri @countSplat) | ||
| $isDeviceGroup = $total -gt 0 -and $devices -eq $total |
There was a problem hiding this comment.
Direct members only, as before: a nested group counts as a non-device member, so a group of groups is not a device group here even when every nested group holds devices. transitiveMembers would change what the finding means and was not measured.
| } | ||
| } | ||
|
|
||
| class FakeGraphException : System.Exception { |
There was a problem hiding this comment.
Untyped properties on purpose: a [System.Net.Http.HttpResponseMessage] type on the property would have to resolve when the file is parsed, and on Windows PowerShell 5.1 System.Net.Http is not loaded until Add-Type runs in BeforeAll.
…equest is sent again Test-IntuneDeployedScript decided whether an assignment's group holds devices from the first page of its members, 20 at most. A mixed group whose first 20 members were devices passed as a device group, and a user-context app assigned to it was flagged as never installing. The check now asks for two counts, every member and the devices among them, with the ConsistencyLevel header that $count needs, so the whole group decides. On the dev tenant $count answers with the number as text; the tests' fake tenant answers the same way. Invoke-IslGraphRequest, the one seam every Graph call goes through, sends a request again when Graph answers 429, 503 or 504: after the Retry-After seconds when the header is there, after 2, 4 and 8 seconds when it is not, three times at most before the failure is thrown as it came. Any other failure is thrown at once, and a page in the middle of -All is retried like any request. The exception's StatusCode and Response are read by name, so the tests throw a stand-in exception with the same members and System.Net.Http response headers rather than needing the Graph module. The seam also takes -Headers.
Summary
Two things about the tenant commands' Graph calls.
Test-IntuneDeployedScriptdecided whether an assignment's group holds devices from the first page of its members, 20 at most, so a mixed group whose first 20 members were devices passed as a device group and a user-context app assigned to it was flagged as never installing. The check now counts the group's members and the devices among them, so the whole group decides. And no Graph request was ever retried: a pre-flight makes two requests per policy, meets Graph's throttle on a tenant with a few hundred policies, and failed on the first 429.Invoke-IslGraphRequest, the one seam every call goes through, now sends a throttled or briefly unavailable request again.Stacked on #18 (base branch); the diff is this change only. Merge #16, #17, #18, then this.
Changes
Public/Test-IntuneDeployedScript.ps1.Test-DeviceGroupasks/groups/{id}/members/$countand/groups/{id}/members/microsoft.graph.device/$countwithConsistencyLevel: eventual; a device group is one where the two numbers agree and are not zero. Checked on the dev tenant on 2026-10-06: both answer with the number as text, and a group of 17 users gives 17 and 0. Two requests per group instead of one, cached per group as before.Private/Invoke-IslGraphRequest.ps1. Takes-Headers. A request that gets 429, 503 or 504 is sent again after theRetry-Afterseconds when the header is there and after 2, 4 and 8 seconds when it is not, three times at most before the failure is thrown as it came; any other failure is thrown at once; a page in the middle of-Allis retried like any request. The status and the response are read from the exception by name, so the tests throw a stand-in with the same members and realSystem.Net.Httpheaders instead of needing the Graph module.-All.Verification
$countbehaviour was measured on the dev tenant through the app's certificate session; the dev tenant has no device group, so the positive case is covered by the fake tenant only.Notes
HttpStatusCodehas no name for 429, so the test builds the status withEnum.ToObject; the seam reads the status as an integer and does not care.$batch. Each tenant command makes two requests per policy and would make one per twenty; the mocks in three test files route by URI and would all need a batch shape.