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

@JobWorker's default autoCompletion can lead to problems #286

Open
korthout opened this issue Dec 7, 2022 · 5 comments
Open

@JobWorker's default autoCompletion can lead to problems #286

korthout opened this issue Dec 7, 2022 · 5 comments
Assignees

Comments

@korthout
Copy link

korthout commented Dec 7, 2022

Nothing explicitly warns users to avoid completing jobs when autoCompletion is enabled.

We should clarify the that exists warning and instruct the user how to resolve the problem.

Why?

Recently, we've seen a few situations where users use the new @JobWorker annotation. This annotation defaults to auto-completing the jobs. Some users may not be aware of this and explicitly complete the job.

When this happens, a warning is logged by the auto-completion logic:

Ignoring the error of type 'NOT_FOUND' during {command=class io.camunda.zeebe.client.impl.command.CompleteJobCommandImpl, job= {...} . Job might have been canceled or already completed.

This warning makes sense but is only helpful for users that are aware of the auto-completion mechanics.

Completing the job twice does not have to be a problem, but it can be when the job is completed asynchronously and with variables. In that case, the explicit job-completion is racing the auto-completion.

@korthout
Copy link
Author

korthout commented Dec 7, 2022

💡 You could pass in a decorated ZeebeClient that does something special when trying to complete the job explicitly, e.g. throw an exception or log a specialized warning with clear instructions.

@korthout korthout changed the title @JobWorker's default autoCompletion conflicts with some user's expectations @JobWorker's default autoCompletion can lead to problems Dec 7, 2022
@Zelldon
Copy link

Zelldon commented Dec 7, 2022

I do something similar in the C# client, using a wrapper and not complete the job if it was completed before https://github.com/camunda-community-hub/zeebe-client-csharp/blob/main/Client/Impl/Worker/JobWorker.cs#L231

@berndruecker
Copy link
Contributor

Ping @falko

@jwulf
Copy link
Member

jwulf commented Feb 7, 2023

I prefer Zelldon's approach. It is: "let the user complete the job when and where they want, and if they don't, do it for them".

Versus having it tell them: "I decide when to complete the job, human".

For the Node.js client, if the user tries to complete the job twice, or complete then fail the job, or some combination of signal multiple outcomes for the same job, I produce an error that warns them that the logic in their handler is faulty. I also made it so that you have to return some job acknowledgement method response from the job handler, to get linting for all code paths that they handled it explicitly.

I chose that over auto-completing in the Node client to make it explicit.

But since we have auto-completing in this client - if the user manually completes the job, they didn't do anything wrong, so we shouldn't throw an error, IMO.

@korthout
Copy link
Author

korthout commented Feb 7, 2023

let the user complete the job when and where they want, and if they don't, do it for them

I like that approach. For completeness, that should also mean: if they fail the job or throw an error on the job manually, don't complete the job for them

@akeller akeller removed this from the 8.2.0 milestone Sep 6, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants