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

Fix transaction error handling #973

Merged
merged 3 commits into from
Aug 7, 2023
Merged

Conversation

itleigns
Copy link
Contributor

@itleigns itleigns commented Aug 7, 2023

Fix #970. When multiple operations are executed together as a transaction in ioredis, the return value takes the following form.

const promise = redis.pipeline().set("foo", "bar").get("foo").exec();
promise.then((result) => {
  // result === [[null, 'OK'], [null, 'bar']]
});

Even if an error occurs in individual operations, it will not be propagated as an error for the entire transaction. To catch errors in each discrete operation, the value in the response is processed in this PR.

@itleigns
Copy link
Contributor Author

itleigns commented Aug 7, 2023

I attempted to create a test, but could not find a way to intentionally provoke an error that would be caught by this fix. This could be achieved by rewriting the internal functions, but I refrained from doing so as there are no unit tests in this repository.

@itleigns
Copy link
Contributor Author

itleigns commented Aug 7, 2023

If multiple operations return errors, only one error is caught. While AggregateError can be used to consolidate multiple errors, it is only supported by Node 15 and later; consequently, this solution was disregarded. (reference)

@evantahler
Copy link
Member

The failing tests are on the main branch, and fixed by #974

@evantahler evantahler added this pull request to the merge queue Aug 7, 2023
Merged via the queue into actionhero:main with commit 8e3eb81 Aug 7, 2023
7 checks passed
@itleigns itleigns deleted the error-handle branch August 8, 2023 01:48
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.

Error handling for enqueueAt
2 participants