search

Particular / NServiceBus

Build, version, and monitor better microservices with the most powerful service platform for .NET
https://particular.net/nservicebus/
Other
2.08k stars 648 forks source link

When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod #1528

Closed indualagarsamy closed 11 years ago

indualagarsamy commented 11 years ago

Scenario:

  1. Endpoint receives a message
  2. There are no handlers found for that message

Current behavior while running within the debugger: Invalid operation is thrown and the message is therefore forwarded to the error queue.

Current behavior when just executing the code: No invalid operation, a warning gets logged. And the message is gone.

foreach (var messageToHandle in messages)
{
    ExtensionMethods.CurrentMessageBeingHandled = messageToHandle;

    var handlers = DispatchMessageToHandlersBasedOnType(builder, messageToHandle).ToList();

    if (!callbackInvoked && !handlers.Any())
    {
        var warning = string.Format("No handlers could be found for message type: {0}", messageToHandle.GetType().FullName);

        if (Debugger.IsAttached)
            throw new InvalidOperationException(warning);

        Log.WarnFormat(warning);
    }

    LogPipelineInfo(messageToHandle, handlers);
}

Question:

Should this behavior be different in these environments? The reason as I understand it is that in dev we want the developer to know this right away so he can fix it and therefore won't be an issue in prod. However, why can't we throw an invalid op exception in prod as well?

Proposed Fix

  1. Log this as an error as opposed to a warning
  2. Treat this similar to deserialization exception and move the message to the error queue without any further retries.
indualagarsamy commented 11 years ago

@johnsimons @andreasohlund @SimonCropp - Thoughts??

johnsimons commented 11 years ago

Yeah, this is inconsistent behavior between dev and prod! Not good!

andreasohlund commented 11 years ago

Ifaik there are scenarios in prod where it might be valid that no handlers fire but I can't remember the details. @udidahan do you remember?

Sent from my iPhone

On 11 sep 2013, at 21:37, Indu Alagarsamy notifications@github.com wrote:

Scenario:

  1. Endpoint receives a message
  2. There are no handlers found for that message

Current behavior while running within the debugger: Invalid operation is thrown and the message is therefore forwarded to the error queue.

Current behavior when just executing the code: No invalid operation, a warning gets logged. And the message is gone.

foreach (var messageToHandle in messages) { ExtensionMethods.CurrentMessageBeingHandled = messageToHandle;

var handlers = DispatchMessageToHandlersBasedOnType(builder, messageToHandle).ToList();

if (!callbackInvoked && !handlers.Any())
{
    var warning = string.Format("No handlers could be found for message type: {0}", messageToHandle.GetType().FullName);

    if (Debugger.IsAttached)
        throw new InvalidOperationException(warning);

    Log.WarnFormat(warning);
}

LogPipelineInfo(messageToHandle, handlers);

} Question:

Should this behavior be different in these environments? The reason as I understand it is that in dev we want the developer to know this right away in prod so he can fix it and in prod its not an issue. However, why can't we throw an invalid op exception in prod as well?

— Reply to this email directly or view it on GitHub.

udidahan commented 11 years ago

I believe the concern with throwing an exception in prod was that this it would trigger all the retry logic, have a perf overhead, ergo a security concern.

I’ve heard several users complain about this over time, so I’m willing to admit I was wrong.

andreasohlund commented 11 years ago

Ok, so lets make it throw at all times. Do we need a way to opt back to the old behavior just in case?

On Thu, Sep 12, 2013 at 11:42 AM, Udi Dahan notifications@github.comwrote:

I believe the concern with throwing an exception in prod was that this it would trigger all the retry logic, have a perf overhead, ergo a security concern.

I’ve heard several users complain about this over time, so I’m willing to admit I was wrong.

From: Andreas Öhlund [mailto:notifications@github.com] Sent: Thursday, September 12, 2013 8:56 AM To: Particular/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod (#1528)

Ifaik there are scenarios in prod where it might be valid that no handlers fire but I can't remember the details. @udidahan do you remember?

Sent from my iPhone

On 11 sep 2013, at 21:37, Indu Alagarsamy notifications@github.com wrote:

Scenario:

  1. Endpoint receives a message
  2. There are no handlers found for that message

Current behavior while running within the debugger: Invalid operation is thrown and the message is therefore forwarded to the error queue.

Current behavior when just executing the code: No invalid operation, a warning gets logged. And the message is gone.

foreach (var messageToHandle in messages) { ExtensionMethods.CurrentMessageBeingHandled = messageToHandle;

var handlers = DispatchMessageToHandlersBasedOnType(builder, messageToHandle).ToList();

if (!callbackInvoked && !handlers.Any()) { var warning = string.Format("No handlers could be found for message type: {0}", messageToHandle.GetType().FullName);

if (Debugger.IsAttached) throw new InvalidOperationException(warning);

Log.WarnFormat(warning); }

LogPipelineInfo(messageToHandle, handlers); } Question:

Should this behavior be different in these environments? The reason as I understand it is that in dev we want the developer to know this right away in prod so he can fix it and in prod its not an issue. However, why can't we throw an invalid op exception in prod as well?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub < https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24296723> . < https://github.com/notifications/beacon/tUwQ8nAYGHIxi8jbkvfo_PQvraReza5PW9rsN_MNinY5nIsrwFRwPaoENPuwXwpQ.gif>

No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3392 / Virus Database: 3222/6632 - Release Date: 09/02/13 Internal Virus Database is out of date.

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306102 .

http://andreasohlund.net http://twitter.com/andreasohlund

udidahan commented 11 years ago

I was thinking to have it just go directly to the error queue in prod – no retries.

andreasohlund commented 11 years ago

We currently don't have a way to control this (other than throwing a SerializationException) so perhaps this is something we need to fix?

On Thu, Sep 12, 2013 at 11:47 AM, Udi Dahan notifications@github.comwrote:

I was thinking to have it just go directly to the error queue in prod – no retries.

From: Andreas Öhlund [mailto:notifications@github.com] Sent: Thursday, September 12, 2013 12:44 PM To: Particular/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod (#1528)

Ok, so lets make it throw at all times. Do we need a way to opt back to the old behavior just in case?

On Thu, Sep 12, 2013 at 11:42 AM, Udi Dahan notifications@github.comwrote:

I believe the concern with throwing an exception in prod was that this it would trigger all the retry logic, have a perf overhead, ergo a security concern.

I’ve heard several users complain about this over time, so I’m willing to admit I was wrong.

From: Andreas Öhlund [mailto:notifications@github.com] Sent: Thursday, September 12, 2013 8:56 AM To: Particular/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod (#1528)

Ifaik there are scenarios in prod where it might be valid that no handlers fire but I can't remember the details. @udidahan do you remember?

Sent from my iPhone

On 11 sep 2013, at 21:37, Indu Alagarsamy notifications@github.com wrote:

Scenario:

  1. Endpoint receives a message
  2. There are no handlers found for that message

Current behavior while running within the debugger: Invalid operation is thrown and the message is therefore forwarded to the error queue.

Current behavior when just executing the code: No invalid operation, a warning gets logged. And the message is gone.

foreach (var messageToHandle in messages) { ExtensionMethods.CurrentMessageBeingHandled = messageToHandle;

var handlers = DispatchMessageToHandlersBasedOnType(builder, messageToHandle).ToList();

if (!callbackInvoked && !handlers.Any()) { var warning = string.Format("No handlers could be found for message type: {0}", messageToHandle.GetType().FullName);

if (Debugger.IsAttached) throw new InvalidOperationException(warning);

Log.WarnFormat(warning); }

LogPipelineInfo(messageToHandle, handlers); } Question:

Should this behavior be different in these environments? The reason as I understand it is that in dev we want the developer to know this right away in prod so he can fix it and in prod its not an issue. However, why can't we throw an invalid op exception in prod as well?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub <

https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24296723>

. <

https://github.com/notifications/beacon/tUwQ8nAYGHIxi8jbkvfo_PQvraReza5PW9rsN_MNinY5nIsrwFRwPaoENPuwXwpQ.gif>

No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3392 / Virus Database: 3222/6632 - Release Date: 09/02/13 Internal Virus Database is out of date.

— Reply to this email directly or view it on GitHub< https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306102>

.

http://andreasohlund.net http://twitter.com/andreasohlund

— Reply to this email directly or view it on GitHub < https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306230> . < https://github.com/notifications/beacon/tUwQ8nAYGHIxi8jbkvfo_PQvraReza5PW9rsN_MNinY5nIsrwFRwPaoENPuwXwpQ.gif>

< http://sgmail.github.com/wf/open?upn=ChcHgk6y5JjWz8A9pw3ECI6jExIK-2Fb4EekdCK5nNNHbTYOZKrh1hItmYpbryfdS9lJPJtlRL-2FMTT-2FIsNGW163U5tM0RVAXwR-2FkO117iFrcf5P8YSy-2F0iiPWGUBB0XYWC9YVrnmXlSOBtOcZpaLUOq7wqoIQ-2BYoI7FYxpimdUmqh-2F2KSnoo8Rh-2BEwxuWizOnwrc0A7aFk32pOqkQ1bWI94w-3D-3D>

No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3392 / Virus Database: 3222/6632 - Release Date: 09/02/13 Internal Virus Database is out of date.

—

Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306423 .

http://andreasohlund.net http://twitter.com/andreasohlund

johnsimons commented 11 years ago

This is really an edge case, IMO lets just throw and let the framework do its retry logic.

On Thursday, September 12, 2013, Andreas Öhlund wrote:

We currently don't have a way to control this (other than throwing a SerializationException) so perhaps this is something we need to fix?

On Thu, Sep 12, 2013 at 11:47 AM, Udi Dahan <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

I was thinking to have it just go directly to the error queue in prod – no retries.

From: Andreas Öhlund [mailto:notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>]

Sent: Thursday, September 12, 2013 12:44 PM To: Particular/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod (#1528)

Ok, so lets make it throw at all times. Do we need a way to opt back to the old behavior just in case?

On Thu, Sep 12, 2013 at 11:42 AM, Udi Dahan <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

I believe the concern with throwing an exception in prod was that this it would trigger all the retry logic, have a perf overhead, ergo a security concern.

I’ve heard several users complain about this over time, so I’m willing to admit I was wrong.

From: Andreas Öhlund [mailto:notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>]

Sent: Thursday, September 12, 2013 8:56 AM To: Particular/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod (#1528)

Ifaik there are scenarios in prod where it might be valid that no handlers fire but I can't remember the details. @udidahan do you remember?

Sent from my iPhone

On 11 sep 2013, at 21:37, Indu Alagarsamy <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>

wrote:

Scenario:

  1. Endpoint receives a message
  2. There are no handlers found for that message

Current behavior while running within the debugger: Invalid operation is thrown and the message is therefore forwarded to the error queue.

Current behavior when just executing the code: No invalid operation, a warning gets logged. And the message is gone.

foreach (var messageToHandle in messages) { ExtensionMethods.CurrentMessageBeingHandled = messageToHandle;

var handlers = DispatchMessageToHandlersBasedOnType(builder, messageToHandle).ToList();

if (!callbackInvoked && !handlers.Any()) { var warning = string.Format("No handlers could be found for message type: {0}", messageToHandle.GetType().FullName);

if (Debugger.IsAttached) throw new InvalidOperationException(warning);

Log.WarnFormat(warning); }

LogPipelineInfo(messageToHandle, handlers); } Question:

Should this behavior be different in these environments? The reason as I understand it is that in dev we want the developer to know this right away in prod so he can fix it and in prod its not an issue. However, why can't we throw an invalid op exception in prod as well?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub <

https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24296723>

. <

https://github.com/notifications/beacon/tUwQ8nAYGHIxi8jbkvfo_PQvraReza5PW9rsN_MNinY5nIsrwFRwPaoENPuwXwpQ.gif>

No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3392 / Virus Database: 3222/6632 - Release Date: 09/02/13 Internal Virus Database is out of date.

— Reply to this email directly or view it on GitHub<

https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306102>

.

http://andreasohlund.net http://twitter.com/andreasohlund

— Reply to this email directly or view it on GitHub <

https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306230>

. <

https://github.com/notifications/beacon/tUwQ8nAYGHIxi8jbkvfo_PQvraReza5PW9rsN_MNinY5nIsrwFRwPaoENPuwXwpQ.gif>

<

http://sgmail.github.com/wf/open?upn=ChcHgk6y5JjWz8A9pw3ECI6jExIK-2Fb4EekdCK5nNNHbTYOZKrh1hItmYpbryfdS9lJPJtlRL-2FMTT-2FIsNGW163U5tM0RVAXwR-2FkO117iFrcf5P8YSy-2F0iiPWGUBB0XYWC9YVrnmXlSOBtOcZpaLUOq7wqoIQ-2BYoI7FYxpimdUmqh-2F2KSnoo8Rh-2BEwxuWizOnwrc0A7aFk32pOqkQ1bWI94w-3D-3D>

No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3392 / Virus Database: 3222/6632 - Release Date: 09/02/13 Internal Virus Database is out of date.

—

Reply to this email directly or view it on GitHub< https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306423>

.

http://andreasohlund.net http://twitter.com/andreasohlund

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306759 .

Regards John Simons NServiceBus

johnsimons commented 11 years ago

No need to opt out as this way is actually safer and no message loss

On Thursday, September 12, 2013, Andreas Öhlund wrote:

Ok, so lets make it throw at all times. Do we need a way to opt back to the old behavior just in case?

On Thu, Sep 12, 2013 at 11:42 AM, Udi Dahan <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>wrote:

I believe the concern with throwing an exception in prod was that this it would trigger all the retry logic, have a perf overhead, ergo a security concern.

I’ve heard several users complain about this over time, so I’m willing to admit I was wrong.

From: Andreas Öhlund [mailto:notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>]

Sent: Thursday, September 12, 2013 8:56 AM To: Particular/NServiceBus Cc: Udi Dahan Subject: Re: [NServiceBus] When a message handler is not found and a message arrives, endpoint exhibits different behaviors in dev & prod (#1528)

Ifaik there are scenarios in prod where it might be valid that no handlers fire but I can't remember the details. @udidahan do you remember?

Sent from my iPhone

On 11 sep 2013, at 21:37, Indu Alagarsamy <notifications@github.com<javascript:_e({}, 'cvml', 'notifications@github.com');>>

wrote:

Scenario:

  1. Endpoint receives a message
  2. There are no handlers found for that message

Current behavior while running within the debugger: Invalid operation is thrown and the message is therefore forwarded to the error queue.

Current behavior when just executing the code: No invalid operation, a warning gets logged. And the message is gone.

foreach (var messageToHandle in messages) { ExtensionMethods.CurrentMessageBeingHandled = messageToHandle;

var handlers = DispatchMessageToHandlersBasedOnType(builder, messageToHandle).ToList();

if (!callbackInvoked && !handlers.Any()) { var warning = string.Format("No handlers could be found for message type: {0}", messageToHandle.GetType().FullName);

if (Debugger.IsAttached) throw new InvalidOperationException(warning);

Log.WarnFormat(warning); }

LogPipelineInfo(messageToHandle, handlers); } Question:

Should this behavior be different in these environments? The reason as I understand it is that in dev we want the developer to know this right away in prod so he can fix it and in prod its not an issue. However, why can't we throw an invalid op exception in prod as well?

— Reply to this email directly or view it on GitHub.

— Reply to this email directly or view it on GitHub <

https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24296723>

. <

https://github.com/notifications/beacon/tUwQ8nAYGHIxi8jbkvfo_PQvraReza5PW9rsN_MNinY5nIsrwFRwPaoENPuwXwpQ.gif>

No virus found in this message. Checked by AVG - www.avg.com Version: 2013.0.3392 / Virus Database: 3222/6632 - Release Date: 09/02/13 Internal Virus Database is out of date.

— Reply to this email directly or view it on GitHub< https://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306102>

.

http://andreasohlund.net http://twitter.com/andreasohlund

— Reply to this email directly or view it on GitHubhttps://github.com/Particular/NServiceBus/issues/1528#issuecomment-24306230 .

Regards John Simons NServiceBus

udidahan commented 11 years ago

Don’t invest too much time in this now.

JeffreyAllenMiller commented 11 years ago

Gentlemen, I really appreciate the time and effort you have put in evaluating my scenario. I have been working with Indu Alagarsamy and Sean Farmar, who have been great to work with by the way, to verify that I am not crazy. I was pulling my hair out for days trying to figure this out. Fortunately, you verified the behavior I reported and my superior will hopefully not think I am a lunatic any longer :)

In my humble opinion this is an important fix because it promotes and solidifies the frameworks claims of being a true TANSACTIONAL messaging system. Administrators or developers for that matter could errantly misconfigure the app.config file during an upgrade for example and point a specific message to the wrong endpoint. In this scenario, we would loose everything. The way it currently works within Visual Studio (i.e. Debugger.IsAttached) is sufficient. I agree with your team that technically it should not perform the retries as it obviously can never work the Nth time it is executed. That said, perhaps a more appropriate solution can be architected that bypasses the retries after the initial patch is implemented.

Again, thanks for your help, anxiously looking for version 4.0.5