Enable-DbaFilestream - Report what the WMI provider actually returned - #10525
Merged
Conversation
Get-FilestreamReturnValue gated its success message on
{ 2147021885 -or 2147945411 -or 0 }, a constant expression that is always true,
so it matched every return code. Since switch runs every matching clause without
a break, this had three effects:
- a documented refusal came back as its own message AND the success message
- default became unreachable, so an unrecognized code was reported as a plain
success and its raw value was lost
- no caller could tell success from failure by looking at the return value
Enable-DbaFilestream then dropped even that, because it only surfaced the value
when -Force was absent, and -Force is the documented non-interactive path that
the examples and every test use. A refused call therefore produced no warning,
no error, and an unchanged instance.
Seen on ci-azure while working #10522: setting FileStream level 2 left the
instance at level 1 on every attempt with nothing reported, and the return code
that would have said why was gone.
The function moves to its own file so it can be tested, and now returns
ReturnValue, Category and Message. Category is Success, Failure or Unknown -
codes in neither list are reported as Unknown with the raw value rather than
guessed at in either direction, since assuming success hides a refusal and
assuming failure invents an error the provider never reported. A null return
value is Unknown too, because switch does not enter any clause for null.
Enable-DbaFilestream stops on a documented refusal and warns on an unrecognized
code, both regardless of -Force. -Force means do not prompt, not do not report
errors. The existing warning about changes needing a restart is unchanged.
Fixes #10524
(do Enable-DbaFilestream, Disable-DbaFilestream, Get-DbaFilestream)
Member
|
I really like this, thank you |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #10524.
The bug
Get-FilestreamReturnValuegated its success message on a constant expression:{ 2147021885 -or 2147945411 -or 0 } { "The requested operation is successful. ..." }$Valuenever appears in it.2147021885 -or 2147945411 -or 0is just$true, so the clause matched every code. And becauseswitchruns every matching clause without abreak, that produced three separate problems:2147024891"Access denied"Access deniedand the success messageFailure/Access deniedUnknown, raw value kept0/ the two21470…codesSuccessdefaultwas unreachable, so an unrecognized code could never fall through to report its own value.Enable-DbaFilestreamthen discarded even that, surfacing the value only when-Forcewas absent — and-Forceis the documented non-interactive path used by the examples and every test. A refused call produced no warning, no error, and an unchanged instance.How it showed up
On a ci-azure RESTART runner while working #10522, setting FILESTREAM level 2 left the instance at level 1 on every attempt with nothing reported. The instance came back with
ServiceShareNamestill set to the instance-name fallback rather than the share name that was passed, so the call had changed nothing at all — and the return code that would have said why was gone.The change
Get-FilestreamReturnValuemoves into its own file underprivate/functions/so it can be tested, and returnsReturnValue,CategoryandMessage.CategoryisSuccess,FailureorUnknown.Codes in neither list are reported as
Unknownwith the raw value rather than guessed at in either direction — assuming success hides a refusal, assuming failure invents an error the provider never reported. A null return value isUnknownfor the same reason, sinceswitchenters no clause at all for$null.Enable-DbaFilestreamnow stops on a documented refusal and warns on an unrecognized code, both regardless of-Force.-Forceshould mean "do not prompt", not "do not report errors". The existing warning that changes need a service restart is unchanged.Testing
Five unit tests cover each category, plus two edge cases worth calling out:
switch ($null)enters no clause, so without an explicit guard this reportedFailurefor a call that never returned anythingInvoke-CimMethodhands back aUInt32, so a comparison that only matchedInt32would push every real call down the unknown pathAgainst a live lab, all three filestream commands still pass:
Enable-DbaFilestream3 integration tests (levels 1 and 2 and the ShareName warning),Disable-DbaFilestreamandGet-DbaFilestreamunchanged. Repo compliance and structure gates: 0 failures.Note
This does not by itself fix the level 2 failure on ci-azure — it makes the provider's reason visible so that can finally be diagnosed. The level 2 test is skipped in #10522 with a pointer to #10524; once this merges it can be unskipped to read the real return code.
🤖 Generated with Claude Code