Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add a panic value to be used by handlers when a message is invalid. #103

Open
jmalloc opened this issue Nov 9, 2019 · 5 comments
Open
Labels
undecided Further discussion is required to how (or whether) to proceed

Comments

@jmalloc
Copy link
Member

jmalloc commented Nov 9, 2019

Generally, handlers should be written expecting pre-validated messages. Nonetheless, they still need a way to signal that a message is invalid and hence that retrying will never work.

@koden-km suggested dogma.MalformedMessage or dogma.InvalidArgument. I think the name mostly needs to communicate what that the message will never work, more so than the reason why.

As Kev rightly pointed out, this panic value needs to be distinct from returning an error value, which will always be retried under the existing engine implementations.

@jmalloc
Copy link
Member Author

jmalloc commented Nov 12, 2019

I don't think it'd be wise to use a singleton, as we do with dogma.UnexpectedMessage. Doing so would require that the panicking code totally discard any contextual information describing what's actually wrong with the message.

I still feel as though a specific type is warranted, though.

Naming wise, perhaps could go for something a little more idiomatic, and perhaps dramatic, like PoisonMessage.

And implementation might simply be:

type PoisonMessage string

With usage like:

panic(PoisonMessage("invalid date"))

@jmalloc jmalloc added this to the v1.0.0 milestone Mar 12, 2020
@jmalloc
Copy link
Member Author

jmalloc commented Mar 12, 2020

@danilvpetrov have you ever come across a situation in Hive/Ax where you have a message that fails forever like described here.. not because of some external problem, but because the message itself is just broken?

@jmalloc
Copy link
Member Author

jmalloc commented Oct 16, 2020

Perhaps this should just be a singleton, and the user can use s.Log() to capture relevant information about why the message is considered invalid, if necessary.

@danilvpetrov
Copy link
Member

danilvpetrov commented Oct 16, 2020

Hey, I just realized that I haven't answered your question since March 13 (the last day before we went WFH). I never had to experience anything like that as I found that the business logic is pretty much incapsulated in its own domain where commands and events are produced within the boundaries of one context.

I did validations in on the API level before executing the business-level command and in integration handlers before producing business events. But I don't recall if I ever had to check the message integrity inside the business level, if it makes sense.

@jmalloc
Copy link
Member Author

jmalloc commented Oct 16, 2020

Yeah that makes sense; and that is definitely the approach we should be taking. However, it is possible that a message can be well-formed (ie, validates OK at the API layer), but is not valid by the time it reaches the domain - meaning that it can not bring about meaningful state change.

This poison message idea was intended to give the way to signal to the handler engine that there is a problem that can't be fixed by retrying. So I guess you can think of it like a practical tool that prevents the engine doing useless work (perpetually retrying), rather than something that's really relevant to the domain logic.

I'm questioning whether we really need this though, so I might just let it sit a little longer until we have some more production experience.

@jmalloc jmalloc removed this from the v1.0.0-rc.1 milestone Oct 16, 2020
@jmalloc jmalloc added the undecided Further discussion is required to how (or whether) to proceed label Oct 26, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
undecided Further discussion is required to how (or whether) to proceed
Projects
Development

No branches or pull requests

2 participants