-
Notifications
You must be signed in to change notification settings - Fork 165
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
db.attachment.getAsStream method does not propagate axios errors to the caller #236
Comments
@glynnbird any thoughts on this issue? |
Fixes issue apache#236. When calling an *AsStream method a client needs to handle errors raised by couch. These errors can include retrieving an attachment that doesn't exist or querying a deleted view.
Fixes issue apache#236. When calling an *AsStream method a client needs to handle errors raised by couch. These errors can include retrieving an attachment that doesn't exist or querying a deleted view.
I too have run into this issue. It's quite poor for the error to go unhandled and stop the running process. I've created a pull request to resolve, or at least start some discussion on the problem. |
* Propagate axios errors to stream client Fixes issue #236. When calling an *AsStream method a client needs to handle errors raised by couch. These errors can include retrieving an attachment that doesn't exist or querying a deleted view. * Removing extra blank line * Correcting README stream function names The examples of several stream functions we not named *AsStream. Co-authored-by: Byron Murgatroyd <[email protected]>
@glynnbird Isn't this issue fixed yet? #246 fixed it and is already merged. |
It seems that error propagation is OK now, but after catching the error the VM doesn't exit. So there must be some unsettled promise. |
@glynnbird @byronmurg Just checked the example code with
Hope that helps. |
Thanks. I'm struggling to get the bottom of this one:
I've tried all combinations of Here's the code in question if anyone wants to suggest some other approach: https://github.com/apache/couchdb-nano/blob/main/lib/nano.js#L382-L393 |
I just dug down and found that if you don't set |
It seems that even keepAlive has to be disabled in the |
Expected Behavior
If I call db.attachment.getAsStream() method and an axios error is thrown the caller should be notified.
Current Behavior
If I call e.g. db.attachment.getAsStream() with a non-existing document id, axios throws an error (404)
The PassThrough instance that db.attachment.getAsStream() returns won't emit any events, so the caller
will never receive the error, an unhandledPromiseRejection warning is raised instead.
Possible Solution
In case of an underlying axios error, the returned PassThrough instance should emit an
error
event.I inspected the corresponding code, nano.js line 343:
couchdb-nano/lib/nano.js
Line 343 in b3f33b6
There should be a catch branch.
Steps to Reproduce (for bugs)
Context
I am trying to create an internal API for reading and writing documents and their attachments.
Your Environment
The text was updated successfully, but these errors were encountered: