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

peer.connect emits error to peer instead of to connection when no connection can be made #1281

Open
1 task done
louwers opened this issue May 26, 2024 · 2 comments
Open
1 task done
Labels
bug unconfirmed not yet verified as an issue

Comments

@louwers
Copy link

louwers commented May 26, 2024

Please, check for existing issues to avoid duplicates.

  • No similar issues found.

What happened?

When no connection can be made, this error is emitted:

`Could not connect to peer ${peerId}`,

The docs state that:

Errors on the peer are almost always fatal and will destroy the peer

Of course, not being able to connect to another peer is definitely not fatal.

How can we reproduce the issue?

No response

What do you expected to happen?

I expected an error to be emitted to the connection as well.

I am not the only one.

If we look at this official(?) example

    connectPeer: (id: string) => new Promise<void>((resolve, reject) => {
        if (!peer) {
            reject(new Error("Peer doesn't start yet"))
            return
        }
        if (connectionMap.has(id)) {
            reject(new Error("Connection existed"))
            return
        }
        try {
            let conn = peer.connect(id, {reliable: true})
            if (!conn) {
                reject(new Error("Connection can't be established"))
            } else {
                conn.on('open', function() {
                    console.log("Connect to: " + id)
                    connectionMap.set(id, conn)
                    resolve()
                }).on('error', function(err) {
                    console.log(err)
                    reject(err)
                })
            }
        } catch (err) {
            reject(err)
        }
    }),

The Promise connectPeer returns will resolve if a peer cannot be connected to, since the error is not emitted to the connection.

Environment setup

Not really relevant.

Is this a regression?

No response

Anything else?

No response

@louwers louwers added bug unconfirmed not yet verified as an issue labels May 26, 2024
@louwers louwers closed this as not planned Won't fix, can't repro, duplicate, stale May 26, 2024
@louwers
Copy link
Author

louwers commented May 26, 2024

This is not a bug but rather something unexpected.

@louwers louwers reopened this May 26, 2024
@louwers
Copy link
Author

louwers commented May 26, 2024

I still think this is a bug, because there is no reliable way to extract which peer failed to connect from the error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug unconfirmed not yet verified as an issue
Projects
None yet
Development

No branches or pull requests

1 participant