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

stop handling then when next is called #1942

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

Conversation

twksos
Copy link

@twksos twksos commented Feb 8, 2023

Pre-Submission Checklist

  • [v] Opened an issue discussing these changes before opening the PR
  • [v] Ran the linter and tests via make prepush
  • [v] Included comprehensive and convincing tests for changes

Issues

Closes:

Summarize the issues that discussed these changes

currently, the async function handling does not meet user's expectation.
a function returning Promise can still have the next param and:

  • it could get called twice.
  • if user send out an error through the next, the promise could still resolve.

Changes

add a local variable to check whether next is called and wrap the next function.

What does this PR do?

  • in chain.js prevent further Promise handling if next is called.
  • add tests

@twksos
Copy link
Author

twksos commented Jul 21, 2023

Hi @mmarchini, could you review this PR when you have time?

@twksos twksos marked this pull request as draft August 9, 2023 00:17
@twksos twksos marked this pull request as ready for review August 9, 2023 00:17
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.

None yet

1 participant