You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2020/11/09 15:36:42 UTC
[GitHub] [couchdb-nano] gyszalai opened a new issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
gyszalai opened a new issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236
<!--- Provide a general summary of the issue in the Title above -->
## 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: https://github.com/apache/couchdb-nano/blob/b3f33b64d75bcd59f8cd9d2b9cd1f90d104b7e94/lib/nano.js#L343
There should be a catch branch.
## Steps to Reproduce (for bugs)
```javascript
const couchUrl = 'http://admin:admin@localhost:5984'
const nano = require('nano')(couchUrl)
const dbName = 'mydb'
const db = nano.use(dbName)
async function run () {
console.log('start')
await nano.db.destroy(dbName)
console.log('db destroyed')
await nano.db.create(dbName)
console.log('db created')
const docId = 'abc1234'
const doc = { _id: docId, something: 'a' }
const data = 'some attachment'
const { rev } = await db.insert(doc)
const attName = 'attName'
console.log('inserting attachment')
const response = await db.attachment.insert(docId, attName, Buffer.from(data, 'utf-8'), 'text/plain', { rev })
console.log('attachment inserted', response)
const attStream = await db.attachment.getAsStream(docId, attName)
console.log('attachment stream returned')
await new Promise((resolve, reject) => {
attStream.on('data', () => console.log('data received:', data))
attStream.on('end', resolve)
attStream.on('error', reject)
})
console.log('attachment read OK')
try {
console.log('try reading attachment of a non-existing doc...')
const attStream2 = await db.attachment.getAsStream('fake_doc_id', attName)
console.log('attachment stream returned')
await new Promise((resolve, reject) => {
attStream2.on('data', () => console.log('data received: ', data))
attStream2.on('end', resolve)
attStream2.on('error', reject)
})
} catch (e) {
console.log('error', e.message)
}
}
run()
```
## Context
I am trying to create an internal API for reading and writing documents and their attachments.
## Your Environment
* Version used: 9.0.1
* Browser Name and version: -
* Operating System and version (desktop or mobile): macOS 10.15.7
* Link to your project:
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-764616889
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] glynnbird commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
glynnbird commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-762172045
Thanks. I'm struggling to get the bottom of this one:
- Nano returns a `stream.PassThrough` object because it wants to be able to return a stream to the user synchronously
- we then run Axios to do the HTTP request to CouchDB. On success the axios stream is piped to the PassThrough stream and all is well. On failure (such as 404), nothing gets piped by an 'error' event is emitted.
I've tried all combinations of `close`, `end`, `destroy` on failure of the axios request but I can't persuade Node.js to kill these objects.
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
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-760770886
@glynnbird Isn't this issue fixed yet? #246 fixed it and is already merged.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-764616889
I just dug down and found that if you don't set `maxSockets: 50` in the defaultHttpAgent options, the VM exits gracefully after receiving and logging the error from axios.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-762088465
@glynnbird @byronmurg Just checked the example code with `why-is-node-running` and found that the followin things prevent the VM from exiting:
```
# ZLIB
/Users/gyszalai/work/github/nano9_bug/node_modules/axios/lib/adapters/http.js:213 - stream = stream.pipe(zlib.createUnzip());
# TTYWRAP
/Users/gyszalai/work/github/nano9_bug/node_modules/axios/lib/adapters/http.js:213 - stream = stream.pipe(zlib.createUnzip());
# TCPWRAP
/Users/gyszalai/work/github/nano9_bug/node_modules/axios/lib/adapters/http.js:195 - var req = transport.request(options, function handleResponse(res) {
/Users/gyszalai/work/github/nano9_bug/node_modules/axios/lib/adapters/http.js:46 - return new Promise(function dispatchHttpRequest(resolvePromise, rejectPromise) {
/Users/gyszalai/work/github/nano9_bug/node_modules/axios/lib/core/dispatchRequest.js:52 - return adapter(config).then(function onAdapterResolution(response) {
```
Hope that helps.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-764619519
It seems that even keepAlive has to be disabled in the `defaultHttpAgent` options
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-761006751
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] gyszalai commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
gyszalai commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-726059368
@glynnbird any thoughts on this issue?
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [couchdb-nano] byronmurg commented on issue #236: db.attachment.getAsStream method does not propagate axios errors to the caller
Posted by GitBox <gi...@apache.org>.
byronmurg commented on issue #236:
URL: https://github.com/apache/couchdb-nano/issues/236#issuecomment-751484142
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.
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org