Skip to content
This repository has been archived by the owner on Apr 8, 2024. It is now read-only.

Emit warning when more than one job action is called for the same job #213

Closed
jwulf opened this issue Mar 8, 2021 · 0 comments
Closed
Assignees
Labels
enhancement New feature or request
Milestone

Comments

@jwulf
Copy link
Member

jwulf commented Mar 8, 2021

The proposed type check is #210 will require the user to return in this case:

 if (listId === undefined) {
      complete.failure(`No list configuration found for ${listName}`);
		// Job failed. Execution should have stopped here, but hasn't
    }
    try {
      await this.emailService.addToList(email, listId);
      complete.success();
    } catch (e) {
      this.logError(e);
      complete.failure(e.message);
    }

That will catch the subtle bug where the addToList side-effect is executed after the job is marked as failed. The programmer intended execution to halt at that point.

Even with the type check in #210, however, it can still be written like this:

 if (listId === undefined) {
      complete.failure(`No list configuration found for ${listName}`);
      // Job failed. Execution should have stopped here, but hasn't	
      // Doesn't need to return, because the type checker sees the subsequent complete.success()
    }
    try {
      await this.emailService.addToList(email, listId);
      return complete.success(); // now returns
    } catch (e) {
      this.logError(e);
      return complete.failure(e.message); // now returns
    }

It now type-checks - all code paths return an action acknowledgement - but it still has the bug.

It would be best to make impossible states impossible to model, but I think this beyond the power of the type system.

This feature is to emit a big red warning to the logger when two job action methods are called for the same job. We can detect that this is a bug, and is definitely impossible behaviour.

I can't see a way to make it impossible to write, but we can have it make noise when it is run.

@jwulf jwulf self-assigned this Mar 8, 2021
@jwulf jwulf added the enhancement New feature or request label Mar 8, 2021
@jwulf jwulf added this to the 1.0.0 milestone Mar 8, 2021
jwulf added a commit to jwulf/zeebe-client-node-js that referenced this issue Mar 19, 2021
Signed-off-by: Josh Wulf <josh@magikcraft.io>
@jwulf jwulf closed this as completed in 0f1a525 May 10, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

1 participant