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

bulkPut's BulkError doesn't specify which keys it failed to put #753

Closed
guanzo opened this issue Oct 14, 2018 · 6 comments
Closed

bulkPut's BulkError doesn't specify which keys it failed to put #753

guanzo opened this issue Oct 14, 2018 · 6 comments

Comments

@guanzo
Copy link

guanzo commented Oct 14, 2018

The BulkError thrown by bulkPut only tells you how many puts failed, but not which ones. Is there any way to find out which keys failed when catching BulkError?

@guanzo guanzo changed the title bulkPut's BulkError should specify which keys failed bulkPut's BulkError doesn't specify which keys it failed to put Oct 14, 2018
@dfahlander
Copy link
Collaborator

No, and that's one thing that is going to change in Dexie 3. Have already created bulkPut methods in another branch (dbcore-middleware-support), that can tell this info. The trick is how to be backward compatible in the API. Probably I will just add another property on BulkError containing a map between positions and the error.

export interface BulkError extends DexieError {
  failures: Error[];
  failuresByPos: {[operationNumber: number]: Error};
}

That way you could enumerate the error and lookup the originating record:

db.bulkPut(items).catch(error => {
   if (error.name === 'BulkError') {
     Object.keys(error.failuresByPos).forEach(pos => {
       console.log(`The item no ${pos} (${JSON.stringify(items[pos])}) failed with error ${error.failuresByPos[pos]}`);
     });
  }
});

What you think about that?

@guanzo
Copy link
Author

guanzo commented Oct 14, 2018

I think that's a good solution. Looking forward to the v3 release and thank you for this wonderful library.

@dfahlander
Copy link
Collaborator

Thanks!

I'll investigate some kind of solution. As it's a major version upgrade it might be ok to change the failures to be the map in itself instead of an array. Just scanned all unit tests and there are no ones referencing failures[0] or failures.length.

Anyhow, any breaking changes will be published in the release notes. Will try to remember to update this issue with the final solution.

@Christilut
Copy link

I just ran into the same problem. Is this possible with v3 now? I'm on 3.0.3

dfahlander added a commit that referenced this issue Jan 12, 2021
* Correction of typings of BulkError.failures - they are a plain array of Error objects.
* Added failuresByPos which contains a map object between the position number of the operation to the actual error.

See #753
@dfahlander
Copy link
Collaborator

I must have missed this issue in 3.0 release. However, it's not a breaking change to introduce failuresByPos property on BulkError so I've done a new PR now that will go into next prerelease (3.1.0-alpha.7). Also noticed that the typings for BulkError.failures is not correct since the runtime value of BulkError.failures are still a plain array of Errors and not an object map (like in 2.x). BulkError.failures is now typed correctly as an Error[] and I've added failuresByPos: {[operationNumber: number]: Error}. A new test for this is also part of the PR #1209.

dfahlander added a commit that referenced this issue Jan 12, 2021
* Correction of typings of BulkError.failures - they are a plain array of Error objects.
* Added failuresByPos which contains a map object between the position number of the operation to the actual error.

See #753
@dfahlander
Copy link
Collaborator

Closing as it is now implemented and will be part of next version after 3.1.0-alpha.6.

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

No branches or pull requests

3 participants