Format or parse error should not throw exception unless ErrorAction.ThrowError is set

[hades200082]

I'm using the following config...

            mailMergeMessage.Config.SmartFormatterConfig.FormatErrorAction = ErrorAction.MaintainTokens;
            mailMergeMessage.Config.SmartFormatterConfig.ParseErrorAction = ErrorAction.MaintainTokens;
            mailMergeMessage.Config.SmartFormatterConfig.CaseSensitivity = CaseSensitivityType.CaseInsensitive;
            mailMergeMessage.Config.SmartFormatterConfig.ConvertCharacterStringLiterals = true;

Yet, when adding {RandomInvalidMergeTag} in the template mailMergeSender.Send(mailMergeMessage, recipients); throws an exception and fails to send the message.

Exception Message: Building of message failed with one or more exceptions.
Inner Exception Message: Variable(s) for placeholder(s) not found: {RandomInvalidMergeTag}

I thought that ErrorAction.MaintainTokens is supposed to ignore this error and just leave them as plain text in the sent message?

Edit: I get the same exception when using ErrorAction.Ignore too
Edit2: Also, shouldn't any error in the sending process call the OnMessageFailure or OnSendFailure events?

[axunonb]

This might be an issue with SmartFormat.Net. Obviously there are cases where exceptions are thrown even when ErrorAction.ThrowError is not set.

Smart.Default.Settings.FormatErrorAction = ErrorAction.MaintainTokens;
Smart.Default.Settings.ParseErrorAction = ErrorAction.MaintainTokens;
var result = Smart.Format("{dummy}", null); // always throws an exception - interpreted as IEnumerable
result = Smart.Format("{dummy}", (object) null); // works

Did you identify more cases than the one mentioned here? Let me know please.

Edit2: Also, shouldn't any error in the sending process call the OnMessageFailure or OnSendFailure events?

will go into a separate issue

[hades200082]

From what I can tell Smart.Default.Settings.FormatErrorAction and Smart.Default.Settings.ParseErrorAction are completely ignored, regardless of what I set them to.

Any invalid merge tags will throw an exception regardless of those settings.

[axunonb]

My remark was leading, sorry. Smart.Default is the static instance of SmartFormat.Net.
For MailMergeLib the relevant instance must be set, i.e. like you wrote in the original post.

new MailMergeMessage).Config.SmartFormatterConfig.ParseErrorAction = ErrorAction.MaintainTokens;
new MailMergeMessage)..Config.SmartFormatterConfig.FormatErrorAction = ErrorAction.MaintainTokens;

So what I understand is that null for the data argument throws an exception. Any other types of data as well? This would be an issue for SmartFormat.Net.
Handling exceptions and events inside MailMergeLib is another issue.

[hades200082]

Sorry, I misunderstood... but no I haven't found any others within our use-case.

What I meant in my previous post is that it doesn't matter what I set new MailMergeMessage().Config.SmartFormatterConfig.ParseErrorAction or new MailMergeMessage().Config.SmartFormatterConfig.FormatErrorAction to... any invalid merge tags will cause MailMergeLib to throw an exception.

This would be an issue for SmartFormat.Net.

Have you already raised it with them or do I need to?

Thanks

[axunonb]

Have you already raised it with them or do I need to?

I also have the honor to take care of SmartFormat.NET :)

[hades200082]

Hi

I see the issue is closed - thanks for that. When will this fix be in the nuget package?

[axunonb]

I'll try to find time over the weekend

[hades200082]

Hi

The latest nuget 5.4.0 doesn't seem to resolve this.

I have the following config:

mailMergeMessage.Config.SmartFormatterConfig.FormatErrorAction = ErrorAction.MaintainTokens;
mailMergeMessage.Config.SmartFormatterConfig.ParseErrorAction = ErrorAction.MaintainTokens;

Yet the call to mailMergeSender.Send(mailMergeMessage, recipients); still throws an exception when invalid/non-existant merge tags are used.

[axunonb]

Hi Lee,
I guess this is more about issue #15
There I asked

I think it is important to note, that as long as a message has formatting or parsing issues coming from SmartFormat, it must never move to MailMergeSender, but throw a MailMergeMessageException. This shall include SmartFormat issues (i.e. inner exceptions if SmartFormat throws errors).
Do you agree on that?

This is how it is implemented, so the MailMergeMessageException is still by design: we never return a half-baked mail message (where even the recipients mail address might not be resolved), but tell the caller what went wrong.
Otherwise it could happen that faulty messages are sent out to (maybe a lot of) recipients. What has changed in this release is better propagating events and being more transparent on what causes a MailMergeMessageException. Also ParsingExecption and FormatErrors are caught and wrapped into a MailMergeMessageException as inner exceptions.
Looks like I don't fully understand what you would like to achieve with a mail message with unresolved tokens maintained.

[hades200082]

ErrorAction.MaintainTokens should not throw an exception on an invalid token... it should just leave that token in place as plain text. If I've set this as the developer then that's what I would expect to happen. It may result in bad looking emails going out but the responsibility for ensuring that correct tokens are used is mine and the users. There are cases where a user might want to put { and } around text and have it print out exactly as written - that should not be up to a library to decide.

Likewise, ErrorAction.Ignore should not throw any exception either, but in this case, the invalid token should be removed. If I've set this as the developer then that's what I would expect to happen. It may result in odd-looking emails with bits missing going out but the responsibility for ensuring that correct tokens are used is mine and the users.

There's another, can't remember the exact name, but its supposed to place the error message in place of the token in the output. If I've set this as the developer (possibly in a training/staging environment) then that's what I would expect to happen. It may result in bad looking emails going out but the responsibility for ensuring that correct tokens are used is mine and the users.

In all of these cases, I as the developer, have made a decision on how the invalid tokens should be handled. So the email sending should not fail due to invalid tokens in the template.

The only time this setting should have any effect on the sending process is if it is set to throw an exception... in that case, an exception should be thrown. This would behave as you seem to have intended.

I've got the library as the email sender for a larger application that allows the end user to build and send emails to dynamic contact groups but where we control the layout of the emails. We allow the users to create templates from a set of pre-defined components. Each component has properties that can be set and some allow the use of tokens or allow free-text into which tokens can be placed.

The sending process is handled by Quartz.Net scheduled jobs. Users can schedule emails to be sent at a specific date and time.

The reasons that this is causing us issues is that we have built in a start/pause/resume system so that if an email starts its send (bear in mind some of the contact groups numbers upwards of 55,000 recipients) and then the app_pool recycles, or the application crashes, the scheduled task will start again once the application is up and running again. In these cases, we keep track of which recipients have already been sent an email (by way of the OnBeforeSend/OnAfterSend handlers) and exclude them from the recipient list when the task starts over.

If the send throws an exception it just leaves the email in limbo as the scheduled task has technically completed but no emails have been sent and the email's status is left at "Scheduled".

[axunonb]

Hm, I get your point. We have the same use case with scheduled tasks here as well, but we clean end user submitted content from whatever we consider as "not allowed", or escape special characters like { and }.

MailMergeLib lets you insert placeholders virtually anywhere: mail addresses, subject, headers, inline attachment, file attachments... So "don't throw MailMergeMessageExceptions" caused by parsing or formatting would mean that these parts would be affected as well by current design. Take a wrong placeholder in an attachment filename, which would finally end up with a FileNotFoundException or an IOException. Even focusing on the plain or HTML message part would not help, because of inline attachments in HTML.

(Should I call it) a possible solution:

1. We extend the MailMergeMessage API with an internal property like DoNotThrowMailMergeMessageExceptions. If this is set to true (defaults to false), all these exceptions will be eaten up silently, never mind their cause.
2. We will add a comment "Use at your own risk, but never in production." :)
3. In a child class of MailMergeMessage this property could be set as desired.

Still hesitating... What do you think?

[hades200082]

Ideally, tokens in the content should be handled separately so that we can allow emails to be sent with invalid tokens (if the user wants to send it like that then let them) but still fail on invalid tokens elsewhere.

It would then also allow for emails that contain code snippets from a tech-blog for example.

Blanket allow/deny exceptions feels icky.

[axunonb]

The latest commit should resolve part your request without causing a breaking change in using the library. Please let me know.
See more comments in at 80bf6d2
In a next step not yet implemented we could extend MailMessageFailureEventArgs in MailMergeSender with an "ignore" return value to the sender, meaning not to throw the MailMergeMessageException.

[axunonb]

With commit 42aae9d the remaining enhancements are included.

1. MailMergeSender.OnMessageFailure delegates may investigate reasons for the message to fail, perform any corrections, and decide whether a MailMergeMessageException shall throw or not. Corrections include modifying the MailMergeMessage, providing a new MimeMessage and return them to the invoker.
2. MailMessageFailureEventArgs.Error (cast to MailMergeMessage.MailMergeMessageException) has the property MimeMessage which contains the incomplete MimeMessage that could be built - but left incomplete to a certain extent due to errors. For your specific request: The MimeMessage from the exception can simply be assigned unchanged to MailMessageFailureEventArgs.MimeMessage. Then set MailMessageFailureEventArgs.ThrowException = false and return from delegate.

For details refer to comments in the commit and the UnitTest Sender_EventsAndSend.Send_With_And_Without_MailMergeMessageException.

@hades200082: Please have a closer look, especially check for missing test cases (PR would be appreciated). The latest modifications allow for an even more fine-grained control than I understood your request. No breaking change to prior versions.
Does this close the issue?

[hades200082]

Thanks for this. I'll be testing this next week and will come back to you then.

[hades200082]

Hi

I've tested this and it works nicely for my use case.

Thanks.

When will this be available on nuget?

[axunonb]

Fine, thanks. Next docs have to be updated, package built and tested. So later this week a new release could be completed.

[hades200082]

If I get some time outside of work I’ll see if I can add anything to this by way of a PR. Thanks for sorting it out.

[axunonb]

Contributions are always welcome!

[axunonb]

Included in new release https://github.com/axuno/MailMergeLib/releases/tag/v5.4.1.0