Error handling

[Goblinlordx]

What version of WebTorrent?
0.102.1

What operating system and Node.js version?
10.7.0

What browser and version? (if using WebTorrent in the browser)
n/a

What did you expect to happen?
Able to handle an error and properly correlate it with event that initiated async task.

What actually happened?
No way to efficiently correlate errors with initial call.

This is much more of a feature request. I would like to hear your thoughts on though. I understand the need for having a "global" catching mechanism for some cases. It seems built like an application instead of a library I can use easily.

For example, if I have several torrents I want to start downloading, I want to add several "torrents". When doing this, there is no way to easily correlate which error events correspond to which call to "add". In doing so, it makes it extremely difficult to manage. It seems to me, it would be far easier to manage if you used the fairly standard "function(err, successResult)" callback signature back to the callback so the caller can receive/handle error appropriately. Better yet, instead of taking a callback simply return a promise which can be rejected if there is an error during the add process specifically (not during the torrent downloading process).

[Goblinlordx]

A note on this, if you do think any of this is a good idea, I would be willing to look into making an appropriate PR to make these adjustments. The larger impact I think would be it would change the API (though it could be done by adding a method instead of changing the existing one).

[KayleePop]

The way to do this with the current API is like this

const client = new WebTorrent()

client.add(torrentId, torrent => {
  torrent.on('error', err => {
    // handle errors for this specific torrent here
  })
})

There are plans for a promise based API in the future.

The reason this isn't an error in the client.add() callback is because callback functions are only supposed to be called once, and errors can happen at any time after the torrent is created.

Multiple errors can happen in the same torrent or client as well.

[Goblinlordx]

The callback isn't called if there is an error parsing (or adding in case of duplicates). client.add can be called multiple times synchronously and there still isn't a way to correlate the parsing error. I understand the need for it in torrent (which exists for some period of time and can have a "stream" of errors and/or other events). Adding, however, happens or it doesn't (transformation from input to a torrent type). If there is an error, it fails and will never be added presumably. This is the type of error I am talking about.

[KayleePop]

Ah, I see the issue. The callback for client.add() is only called after the torrent's metadata and store are ready (after the ready event)

If you only register the error handler in the callback, then any errors that happen before the torrent is ready won't be captured.

You can register the error handler immediately using the return value of client.add() like this though.

const client = new WebTorrent()

const torrent = client.add('invalid torrent id')
torrent.on('error', error => {
  console.log('error caught!') //handle errors here
})

Here's a jsfiddle with this code https://jsfiddle.net/9tks8vw4/1/

[Goblinlordx]

This works in only very strictly limited cases and is very difficult to manage. If I, for example, have 20 torrents I would like to add to a given client. When I add each one, I would need to proceed to the next one only after either an error or after the add(torrent, cb) cb function is called (because there is only 1 global error handler callback). This also assumes that there is no other possible error the client can emit after a torrent has been successfully added.
This seems extremely difficult to handle using the current API. If 3 torrents were added successfully, and then an error which is unrelated to add is emitted from the client while waiting for either an error (client.on('error', cb)) or success (add('',cb)). There seems to be a huge problem that these 2 things are completely disconnected and there is no way to reliably be notified when calling add.

Again, I would be willing to modify or update the existing API in a PR to add a error callback. Looking at the code though... everything seems very tightly coupled with event emitting and there seems to be no error handling of any given function from the caller and instead they are handled inside and have a hidden dependency on a "global" error callback. I think any change would involve moving all event emitting to something which calls the functions instead of inside the functions themselves. I am not sure a PR would get through with such heavy modifications. Also, I think I would need to change all functions to take a success/failure callback parameter so that each step is individual and doesn't know about the next step. After that, they should no longer have a dependency on event emitting and could be changed to promises relatively easily.

[KayleePop]

If I, for example, have 20 torrents I would like to add to a given client. When I add each one, I would need to proceed to the next one only after either an error or after the add(torrent, cb) cb function is called.

I don't see why this is the case. There's a separate handler for torrent errors torrent.on('error', err => {}). It will only be called for that specific torrent and no others, so you could add 20 torrents to one client all in parallel and still know which ones failed to be added and which ones succeeded.

Have you seen https://webtorrent.io/docs? Search for client.on('error' and torrent.on('error'

This also assumes that there is no other possible error the client can emit after a torrent has been successfully added.

Since torrent errors are fatal, I don't think the error handler would be called multiple times, but if so, then you can use torrent.once('error', () => {}) which stops listening after the first error. https://nodejs.org/api/events.html#events_emitter_once_eventname_listener

[KayleePop]

Errors aren't just emitted in response to function calls. There are lots of on going processes like network sockets, interactions with peers, and file system access that can all experience errors but don't really have a callback to send them to.

[Goblinlordx]

I don't see why this is the case. There's a separate handler for torrent errors torrent.on('error', err => {})

I already stated this is a concern for before the add success callback is called (we don't have torrent yet). After we have torrent I already stated, I understand completely why there is a stream of errors needed and not a "single" error callback function.

Since torrent errors are fatal, I don't think the error handler would be called multiple times, but if so, then you can use torrent.once('error', () => {}) which stops listening after the first error.

This wouldn't work very well. If there is an error all error handlers would be called for the error event.

What I would specifically like to handle properly is errors during add processing. Nothing else is relevant. The solutions you have presented (once and setting on) don't work. For example, if 10 torrents were added and still being processed and 1 more was added that triggered error, it would then call all the error callbacks that were set.

Errors aren't just emitted in response to function calls. There are lots of on going processes like network sockets, interactions with peers, and file system access that can all experience errors but don't really have a callback to send them to.

I do not see why this should not be the case with add. add is a succeed or fail operation. If there is a single error during add it fails and there is no further processing (not a stream). If it succeeds, it succeeds and has a single return value passed through a callback (not a stream). The value returned is a torrent type which is a stream.

[Goblinlordx]

Here is a simple example of what I am talking about:
http://jsfiddle.net/9tks8vw4/7/

I used on so you could see they are all called for each error. Using once doesn't work because they are all called immediately with the first error.

[KayleePop]

Here's the fix https://jsfiddle.net/9tks8vw4/9/

[Goblinlordx]

Hmmm... ok I obviously didn't look at the return type when looking at the code. Also, it looks like this is in the documentation but not shown in any examples or anything which may have been the source of my confusion

From docs:

If you want access to the torrent object immediately in order to listen to events as the metadata is fetched from the network, then use the return value of client.add. If you just want the file data, then use ontorrent or the 'torrent' event.

I think it would be good to at least annotate the return type. Maybe an example with error handling on add would be helpful (it seems like someone everyone would need initially using this as a library).

I apologize for any confusion =/.

[KayleePop]

Happy to help! 😄

It's not intuitive you're right. You could submit a PR for the docs if you wanted. https://github.com/webtorrent/webtorrent/tree/master/docs

Error handling would be a good addition to https://github.com/webtorrent/webtorrent/blob/master/docs/get-started.md I think