multiformats/js-multiaddr

bug: new kubo/ipfs/libp2p protocols break JS ecosystem

SgtPooki opened this issue · 2 comments

SgtPooki commented

Describe the bug
Multiple issues across the JS ecosystem when new protocols are added.

js-multiaddr/src/protocols-table.ts

Lines 4 to 55 in 6cfb7ad

export const names: Record<string, Protocol> = {}
export const codes: Record<number, Protocol> = {}
export const table: Array<[number, number, string, boolean?, boolean?]> = [
[4, 32, 'ip4'],
[6, 16, 'tcp'],
[33, 16, 'dccp'],
[41, 128, 'ip6'],
[42, V, 'ip6zone'],
[43, 8, 'ipcidr'],
[53, V, 'dns', true],
[54, V, 'dns4', true],
[55, V, 'dns6', true],
[56, V, 'dnsaddr', true],
[132, 16, 'sctp'],
[273, 16, 'udp'],
[275, 0, 'p2p-webrtc-star'],
[276, 0, 'p2p-webrtc-direct'],
[277, 0, 'p2p-stardust'],
[280, 0, 'webrtc'],
[290, 0, 'p2p-circuit'],
[301, 0, 'udt'],
[302, 0, 'utp'],
[400, V, 'unix', false, true],
// `ipfs` is added before `p2p` for legacy support.
// All text representations will default to `p2p`, but `ipfs` will
// still be supported
[421, V, 'ipfs'],
// `p2p` is the preferred name for 421, and is now the default
[421, V, 'p2p'],
[443, 0, 'https'],
[444, 96, 'onion'],
[445, 296, 'onion3'],
[446, V, 'garlic64'],
[448, 0, 'tls'],
[460, 0, 'quic'],
[461, 0, 'quic-v1'],
[465, 0, 'webtransport'],
[466, V, 'certhash'],
[477, 0, 'ws'],
[478, 0, 'wss'],
[479, 0, 'p2p-websocket-star'],
[480, 0, 'http'],
[777, V, 'memory']
]
// populate tables
table.forEach(row => {
const proto = createProtocol(...row)
codes[proto.code] = proto
names[proto.name] = proto
})

To Reproduce
Steps to reproduce the behavior:

  1. pull down repo ipfs/ipfs-webui
  2. git checkout ea89e7f
  3. rm patches/multiaddr+8.1.2.patch
  4. npm install
  5. start ipfs daemon using kubo 0.18.1 or later in another terminal
    • Note the port number in the line API server listening on /ip4/127.0.0.1/tcp/<kuboPort>
  6. run KUBO_PORT_2033_TEST=<kuboPort> npm run test:unit -- src/bundles/identity.test.js
  7. Note the error "no protocol with name: quic-v1"

Expected behavior
Looking up unknown protocols does not propagate errors, nor cause requests to fail. We should fallback to supported/known protocols

Additional context

Potential Solutions

  1. Pull out protocols-table.js into it's own package. Keep it at v1.0.x, there are NO breaking changes, only protocols added. Any JS package stays up to date by depending directly on the table
  2. Stop throwing errors from

    js-multiaddr/src/protocols-table.ts

    Lines 79 to 95 in 6cfb7ad

    export function getProtocol (proto: number | string): Protocol {
    if (typeof proto === 'number') {
    if (codes[proto] != null) {
    return codes[proto]
    }
    throw new Error(`no protocol with code: ${proto}`)
    } else if (typeof proto === 'string') {
    if (names[proto] != null) {
    return names[proto]
    }
    throw new Error(`no protocol with name: ${proto}`)
    }
    throw new Error(`invalid protocol id type: ${typeof proto}`)
    }
    , because different packages in the ecosystem are not handling this properly.
  3. backport any protocol-table changes for all multiaddr versions (No one wants to do this)
2color commented

Is this problem showing up in packages using an older version of js-multiaddr? If so, what's stopping an upgrade to the latest version of js-multiaddr?

SgtPooki commented

Is this problem showing up in packages using an older version of js-multiaddr? If so, what's stopping an upgrade to the latest version of js-multiaddr?

I’ll have to investigate this to confirm. Maybe this problem is solved in the latest versions.

However, I imagine it would still be a problem if anyone is out of date on any package required, which can be really confusing.

This issue was intended to bring attention to this error and that the only viable solutions are:

  1. Update all packages
  2. Patch your old version of the package
  3. Ensure all packages in the lib-space (the ones we own) guard against new and unexpected failures.

However, all of those mean someone(consumers, consumers, owners; respectively) is responsible for work that we shouldn't require of them.

Number 1 is improbable to expect all consumers to be able to do.
Number 2 is a localized fix for a global problem.
Number 3 is a not-DRY and highly error-prone solution thats not practical nor maintainable.

Ideally, we could solve this problem in a future proof way where

  1. newly added protocols don’t need guarded against in every lib consuming this one
  2. Consumers with old deps arent forced between a rock and a hard place
  3. Devs don’t need to solve this error everytime we utilize this package.

I’m not 100% sure what the fix for this should be right now but I know we need to a better solution. My gut instinct for a solution is converting the fatal error to a logged but passable one.