Fix recorded requests skipped by request logger #346
Conversation
- When proxy is enabled the recorded requests are mistaken (IMO) for admin requests and skipped
| @@ -99,7 +100,7 @@ private async Task InvokeInternal(IContext ctx) | |||
| return; | |||
| } | |||
|
|
|||
| logRequest = !targetMapping.IsAdminInterface; | |||
| logRequest = !targetMapping.IsAdminInterface || targetMapping.Provider is ProxyAsyncResponseProvider; | |||
There was a problem hiding this comment.
The mapping class already has this code:
public bool IsAdminInterface => Provider is DynamicResponseProvider || Provider is DynamicAsyncResponseProvider || Provider is ProxyAsyncResponseProvider;So no need to add this here?
There was a problem hiding this comment.
That's the root cause of the problem (there is negation). In this check it is actually assumes that proxy response is NOT admin interface in order to log it. I have tried to remove that condition on mapping but it actually conflicts with Admin interface. There is a test for that - FluentMockServer_Proxy_Should_Not_overrule_AdminMappings. So it is kind of workaround to still log proxy requests.
There was a problem hiding this comment.
Ah yes, Sorry.
In that case. Maybe it's better to to add a property to the mapping class, like this:
public bool LogRequest => !(Provider is DynamicResponseProvider || Provider is DynamicAsyncResponseProvider).There was a problem hiding this comment.
What about smth like:
public bool IsProxyRequest => Provider is ProxyAsyncResponseProvider
There was a problem hiding this comment.
But then you still need to check if it's not an admin request.
There was a problem hiding this comment.
In the MiddleWare - yes. Let me update the PR
src/WireMock.Net/Mapping.cs
Outdated
| @@ -46,7 +46,10 @@ public class Mapping : IMapping | |||
| public bool IsStartState => Scenario == null || Scenario != null && NextState != null && ExecutionConditionState == null; | |||
|
|
|||
| /// <inheritdoc cref="IMapping.IsAdminInterface" /> | |||
| public bool IsAdminInterface => Provider is DynamicResponseProvider || Provider is DynamicAsyncResponseProvider || Provider is ProxyAsyncResponseProvider; | |||
| public bool IsAdminInterface => Provider is DynamicResponseProvider || Provider is DynamicAsyncResponseProvider; | |||
There was a problem hiding this comment.
It is too dangerous to update this code because .IsAdminInterface is used in more places in the application.
I think the best solution is to add a property LogRequest as proposed earlier.
There was a problem hiding this comment.
It actually logs both, request and response. Is LogMapping better name for this property?
Codecov Report
@@ Coverage Diff @@
## master #346 +/- ##
==========================================
+ Coverage 80.25% 80.26% +<.01%
==========================================
Files 111 111
Lines 4442 4443 +1
==========================================
+ Hits 3565 3566 +1
Misses 877 877
Continue to review full report at Codecov.
|
|
Thanks. |
When proxy is enabled the recorded requests are mistaken (IMO) for admin interface requests and skipped in LogEntries. Actually none of requests logged in proxy mode. It is valuable to monitor that while recording.