errors in PullTransfers are issued after end (or close)

[dfriederich]

When trying to use the PullTransfer returned by a DeviceClient.pull, I did not find a way to reliably report errors.
The issue is that while errors are reported, all the reliably sent events in the PullTransfer class are executed first.
Hence there is no way to for example fail a result promise reliably, as there is no event which I can wait for.
The attached test code results in this output:

`d.friederich@xdfriederich:.../src/adbkit$node ./dist/src/pull_error_order.js
pull_simple_async: /data/local/tmp/file_which_does_not_exist
event: stream resume
event: stream prefinish
event: stream end
event: stream finish
event: stream close
pull done

event: stream error
error argument: FailError: Failure: 'open failed: No such file or directory'
sleep done
pull_error_order.zip

pull_async: /data/local/tmp/file_which_does_not_exist
event: stream resume
event: stream prefinish
event: stream end
event: stream finish
event: stream close
pull done

event: stream error
error argument: FailError: Failure: 'open failed: No such file or directory'
`
Or in other words, the optional error event is sent after the end, finish and close events. Hence there is no event I can wait for reliably.
Is there an event or some other way to know that the operation is done and no error can happen anymore? I looked but did not find anything. I think the current order of events is incorrect, the error has to occur before some other event to make it usable if we need to associate the error with a specific pull operation.

[pimterry]

I've just run into this too: pull(nonExistantFile) returns a stream that fires end then error events.

This isn't correct node stream behaviour - you should either fire one or the other. Firing an end event means that the entire stream has been read successfully. That makes it very difficult using normal APIs to tell the difference between an empty file or a missing file. In both cases, if you pass this stream to correct stream-to-buffer code, you'll get a successful empty buffer out the end, even if the read actually failed.

It looks like this was introduced by this change: b4b8235

[dfriederich]

The leaked promise returned by readnext() in the b4b8235 change also causes the following warning, once per pull.

2024-07-24T22:30:41.053Z root ERROR (node:3184086) Warning: a promise was created in a handler at node:internal/async_hooks:130:17 but was not returned from it, see http://goo.gl/rRqMUw
    at Promise (/work_ssd/dfriederich2/src/sokatoa/main/app/applications/electron-app/lib/backend/vendors-node_modules_drivelist_build_Release_drivelist_node-node_modules_devicefarmer_adbkit_-f56702.js:49689:10)
2024-07-24T22:30:41.066Z root ERROR (node:3184086) Warning: a promise was created in a handler at node:internal/async_hooks:130:17 but was not returned from it, see http://goo.gl/rRqMUw
    at Promise (/work_ssd/dfriederich2/src/sokatoa/main/app/applications/electron-app/lib/backend/vendors-node_modules_drivelist_build_Release_drivelist_node-node_modules_devicefarmer_adbkit_-f56702.js:49689:10)
2024-07-24T22:30:41.171Z root ERROR (node:3184086) Warning: a promise was created in a handler at node:internal/async_hooks:130:17 but was not returned from it, see http://goo.gl/rRqMUw
    at Promise (/work_ssd/dfriederich2/src/sokatoa/main/app/applications/electron-app/lib/backend/vendors-node_modules_drivelist_build_Release_drivelist_node-node_modules_devicefarmer_adbkit_-f56702.js:49689:10)

Clearly failed operations are not reported, and this can cause unhandled rejected promises :-(.