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

Empty Success or Failure return on saga step causes failure on Redis adapter #3

Open
vessi opened this issue Oct 12, 2021 · 0 comments · May be fixed by #4
Open

Empty Success or Failure return on saga step causes failure on Redis adapter #3

vessi opened this issue Oct 12, 2021 · 0 comments · May be fixed by #4

Comments

@vessi
Copy link

vessi commented Oct 12, 2021

Hi there.

Issue

Noticed that saga with Redis adapter fails if step returns empty Success or Failure value. This is caused by next equality

Success().value! == Dry::Monads::Unit

Script to reproduce

class BaseStep
  include Dry::Monads[:result]

  def call(context)
    Success()
  end
end

class Step1 < BaseStep; end
class Step2 < BaseStep; end

redis = ConnectionPool.new { Redis.new }
redis_adapter = Novel::RepositoryAdapters::Redis.new(connection_pool: redis)
repository = Novel::SagaRepository.new(adapter: redis_adapter)

saga = Novel.compose(logger: Logger.new(STDOUT), repository: repository, timeouts: 5) # timeout in seconds
      .build(name: :test_saga)
      .register_step(:step1, activity: { command: Step1.new },              compensation: { command: Step1.new })
      .register_step(:step2, activity: { command: Step2.new, async: true }, compensation: { command: Step2.new })
      .build

saga.call()

Expected result:

Successful saga execution

Actual result:

TypeError: singleton can't be dumped
from /usr/local/bundle/gems/novel-0.3.0/lib/novel/repository_adapters/redis.rb:16:in `dump'

Reason

Unit is defined as singleton instance of Object, which means it can't be dumped by Marshal.dump which is used by Redis RepositoryAdapter here.

Potential solution

I think it can be avoided by introducing custom marshalling of Novel::Context but context serialiser location, on my opinion, should be in adapter. I'll try to resolve the issue and create PR for it.

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 a pull request may close this issue.

1 participant