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

Directives body opt #189

Open
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

livius-ungureanu
Copy link

@livius-ungureanu livius-ungureanu commented Feb 10, 2020

Problem:

The app should be signaled somehow by op-rabbit about an unparsable item delivered by RabbitMQ in order to take specific actions.(e.g. raise specific service logs, increment specific stats etc..)

Solution:

To address this case we'd need a new directive in order to signal in invalid message(i.e. unparsable message) to the client code:

 bodyEither(as[JobDescription]) { jobDescriptionEither => // alarm the reason and take specific action

@livius-ungureanu
Copy link
Author

livius-ungureanu commented Feb 10, 2020

Note: currently this is on top of add more tls support PR

@livius-ungureanu
Copy link
Author

livius-ungureanu commented Feb 11, 2020

I'd like to also add in DirectivesSpec a test to prove that when the unmarashalling of the body fails then a Either.Left is produced with the cause.
Unfortunately, the only unmarshallers visisble in DirectivesSpec are UTF8StringMarshaller and BinaryMarshaller so it hard to make marshaling fail.

In order to achieve this the import com.spingo.op_rabbit.PlayJsonSupport._ needs to be resolved in DirectivesSpec which currently does not happen due to project structure. If there is a workarround to this I'd be happy to also add Either.Left test. Though I tested successfully the Either.Left in a mini app where I could import anything I needed.

@timcharper
Copy link
Member

Op-rabbit already supports the | operator. One could do

(body(as[JobDescription]).map(Right(_))) | (body(as[String]).map(Left(_)))

Also, op-rabbit has support for publishing failed items to an error queue, I believe, isn't this one of the strategies? Maybe this would be a way to handle failed parsing?

@livius-ungureanu
Copy link
Author

 bodyEither(as[JobDescription]) { jobDescriptionEither => // alarm the reason and take specific action

would make the library user a bit happier because:

  1. the low level (parsing) exception is captured in the jobDescriptionEither so it can be logged
  2. more ergonomic and user friendly.

@scirstoiu
Copy link

I am also interested in this feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants