dohertj2/mxaccessgw
1
0
Fork 0
Code Issues 4 Pull Requests 2 Actions Packages Projects Releases Wiki Activity

AlarmAckWriter: broad ArgumentException filter reports programming errors as auth/write failures #164

New Issue
Open
opened 2026-10-20 07:04:21 -04:00 by dohertj2 · 1 comment
dohertj2 commented 2026-10-20 07:04:21 -04:00
Owner
Copy Link
Copy Source

AlarmAckWriter classifies interop failures with IsInteropFailure (src/ZB.MOM.WW.MxGateway.Worker/MxAccess/AlarmAckWriter.cs:252), which matches ExternalException, ArgumentException and UnauthorizedAccessException. The ArgumentException arm is needed because MXAccess's interop maps E_INVALIDARG to ArgumentException. A wrong password or unknown user makes AuthenticateUser throw ArgumentException (0x80070057), as seen live on windev on 2026-10-19.

Problem: the filter also catches programming-error ArgumentExceptions (including ArgumentNullException) raised inside the same try blocks (:114 AuthenticateUser, :182 AddItem/Advise/Write). For example, a null VerifyUser would surface to the client as MXACCESS_FAILURE "Operator authentication failed … (HRESULT 0x80070057)" instead of an obvious bug.

Suggested fix: narrow the filter to exceptions thrown by the COM call itself. Validate arguments before the try block so our own ArgumentExceptions can't reach the catch. Alternatively, match ArgumentException only when it carries HResult == E_INVALIDARG and is not an ArgumentNullException/ArgumentOutOfRangeException. At minimum, add a comment explaining why the broad filter is deliberate.

Found in the final review of PR #162 (feat/ackmsg-as-operator); left unfixed by decision as Minor.

https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT

`AlarmAckWriter` classifies interop failures with `IsInteropFailure` (`src/ZB.MOM.WW.MxGateway.Worker/MxAccess/AlarmAckWriter.cs:252`), which matches `ExternalException`, `ArgumentException` and `UnauthorizedAccessException`. The `ArgumentException` arm is needed because MXAccess's interop maps `E_INVALIDARG` to `ArgumentException`. A wrong password or unknown user makes `AuthenticateUser` throw `ArgumentException` (0x80070057), as seen live on windev on 2026-10-19. **Problem:** the filter also catches programming-error `ArgumentException`s (including `ArgumentNullException`) raised inside the same try blocks (`:114` AuthenticateUser, `:182` AddItem/Advise/Write). For example, a null `VerifyUser` would surface to the client as `MXACCESS_FAILURE` "Operator authentication failed … (HRESULT 0x80070057)" instead of an obvious bug. **Suggested fix:** narrow the filter to exceptions thrown by the COM call itself. Validate arguments before the try block so our own `ArgumentException`s can't reach the catch. Alternatively, match `ArgumentException` only when it carries `HResult == E_INVALIDARG` and is not an `ArgumentNullException`/`ArgumentOutOfRangeException`. At minimum, add a comment explaining why the broad filter is deliberate. Found in the final review of PR #162 (feat/ackmsg-as-operator); left unfixed by decision as Minor. https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT
dohertj2 commented 2026-10-20 07:29:01 -04:00
Author
Owner
Copy Link
Copy Source

Fixed on feat/ackmsg-as-operator (PR #162) in 219611c + 463748e. Each MXAccess call (AuthenticateUser, AddItem, Advise, Write) now runs in its own narrow interop filter that wraps the failure in a private AckInteropException; the outer catches catch only that. Argument checks, caching and the pump step now propagate as bugs. (A first attempt filtered on HResult == E_INVALIDARG, but a managed ArgumentException already defaults to 0x80070057, so that was dropped.) windev Worker.Tests 585/0. Closes when PR #162 merges.

https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT

Fixed on feat/ackmsg-as-operator (PR #162) in 219611c + 463748e. Each MXAccess call (AuthenticateUser, AddItem, Advise, Write) now runs in its own narrow interop filter that wraps the failure in a private AckInteropException; the outer catches catch only that. Argument checks, caching and the pump step now propagate as bugs. (A first attempt filtered on HResult == E_INVALIDARG, but a managed ArgumentException already defaults to 0x80070057, so that was dropped.) windev Worker.Tests 585/0. Closes when PR #162 merges. https://claude.ai/code/session_016J6isfcgqy4SB2RK3ZvFvT
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
Labels
Clear labels
area:auth
Authentication and authorization
 area:client-dotnet
.NET client library
 area:client-go
Go client library
 area:client-java
Java client library
 area:client-python
Python client library
 area:client-rust
Rust client library
 area:contracts
Protocol and generated contract work
 area:dashboard
Blazor dashboard work
 area:docs
Documentation
 area:gateway
Gateway server work
 area:tests
Automated and live tests
 area:worker
MXAccess worker process work
 blocked
Blocked by dependency
 priority:p0
Must-have / blocking
 priority:p1
High priority
 priority:p2
Normal priority
 type:docs
Documentation work
 type:feature
Feature implementation
 type:infra
Infrastructure and scaffolding
 type:test
Test work
No Label
Milestone
No items
No Milestone
Projects
Clear projects
No project
Assignees
Clear assignees
dohertj2 (dohertj2)
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: dohertj2/mxaccessgw#164
Delete Branch "%!s()"

Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?