Repository navigation
Dbe/fix soap fault marks endpoint down - #74
Conversation
A business error returned by the ERP (e.g. NAV "wrong price" or locking errors) arrives as a FaultException via the Dynamicweb connector service. It was classified as a connectivity error, marking the endpoint as down for all users/sessions until the next ping interval elapsed. SOAP faults now fail only the current request and are not retried. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
MatthiasSort
left a comment
MatthiasSort
left a comment
There was a problem hiding this comment.
Found a regression and a gap while reviewing this: the new IsSoapFault check removes the only retry safety net on the live calculate-order path, and doesn't cover SOAP faults delivered through the plain-HTTP EndpointConnector path (so the "SOAP fault marks endpoint down" bug this PR targets can still reproduce there). Left inline comments with the specifics and traced call chains.
One more, not inline since it's outside the touched diff lines: the new using System.ServiceModel; / FaultException usage isn't backed by an explicit package reference in this project — it only resolves because Dynamicweb.DataIntegration 10.21.5 transitively pulls in System.ServiceModel.Http. Worth adding an explicit PackageReference for System.ServiceModel.Http (or similar) so this doesn't silently break on a future Dynamicweb.DataIntegration version bump.
- Only treat a fault as an ERP business error when the ERP itself returned it: a FaultException passed through by the connector service, or a connector-service ExceptionDetail chain containing an HTTP 500 WebException (BC OAuth path). Connector-service internal faults (InternalServiceFault, e.g. ERP unreachable, timeouts, config errors) still mark the endpoint down. - Keep the existing retry behavior for business faults. - Report business faults as HttpStatusCode.InternalServerError, matching the Endpoint path, so GetProductsInfo callers apply the same 500 handling. - Add explicit System.ServiceModel.Primitives package reference. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
@MatthiasSort thanks for the review. All points are addressed in 8dc1087 (replies inline). On the package reference from the review summary: I added an explicit |
2 participants
No description provided.