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

web3.eth.subscribe("newBlockHeaders", null, ...) throws #3159

Closed
Velenir opened this issue Oct 25, 2019 · 6 comments
Closed

web3.eth.subscribe("newBlockHeaders", null, ...) throws #3159

Velenir opened this issue Oct 25, 2019 · 6 comments
Assignees
Labels
1.x 1.0 related issues Types Incorrect or missing types

Comments

@Velenir
Copy link
Contributor

Velenir commented Oct 25, 2019

Description

Types for Eth.subscribe seem to allow null as the second parameter for all subscriptions:

    subscribe(
        type: 'newBlockHeaders',
        options?: null,
        callback?: (error: Error, blockHeader: BlockHeader) => void
    ): Subscription<BlockHeader>;

But web3.eth.subscribe("newBlockHeaders", null, console.log) breaks with

Uncaught Error: Invalid number of parameters for "null". Got 1 expected 1!

On the other hand web3.eth.subscribe("newBlockHeaders", console.log), which works as expected, isn't allowed by new types, though it used to be in @types/web3

So either types are wrong, or subscribe() function, or both.

Expected behavior

Nothing breaks, both

web3.eth.subscribe("newBlockHeaders", null, console.log)
web3.eth.subscribe("newBlockHeaders", console.log)

work and allowed by TS

Actual behavior

web3.eth.subscribe("newBlockHeaders", null, console.log)

throws

Uncaught Error: Invalid number of parameters for "null". Got 1 expected 1!

web3.eth.subscribe("newBlockHeaders", console.log)

gives

No overload matches this call.
  The last overload gave the following error.
    Type '{ (message?: any, ...optionalParams: any[]): void; (message?: any, ...optionalParams: any[]): void; (message?: any, ...optionalParams: any[]): void; }' has no properties in common with type 'LogsOptions'.ts(2769)
index.d.ts(99, 5): The last overload is declared here.

Steps to reproduce the behavior

1.Initialize web3 with a provider
2. Try to subscribe

Error Logs

Gists

https://codesandbox.io/s/withered-surf-y9ccp
(types don't work here though)

Versions

  • web3.js: 1.2.2
  • browser: Chrome 79.0.3938.0

Btw, I can't find supportsSubscripts on web3.currentProvider in v1.2.2, but it's there in 2.0.0-alpha.1

@cgewecke
Copy link
Collaborator

cgewecke commented Oct 27, 2019

@Velenir Thanks for opening and for describing the problem so well. Have opened #3162 to address this.

In 1.x web3-eth/src/index.js, syncing, newBlockHeaders, and pendingTransactions do not expect any options param. The 1.x documentation for these subscriptions is correct.

re: supportsSubscriptions()

As currently written, the result of this method is true when the connection is ws or ipc, and false when http. In turn, provider.on is unavailable over http, but many Web3 methods actually provide an event emitter interface over http as well. So, it's a bit confusing / not perfect. For more context, the origin of supportsSubscription is #1015.

There's also ongoing discussion about how ethereum provider features support should really be defined at the EIP 1193 Discussion if you're interested in this topic.

@Velenir
Copy link
Contributor Author

Velenir commented Oct 27, 2019

Thank you for the quick response. Happy to know it's types only which are faulty.

re:re: supportsSubscriptions()
web3.currentProvider.supportsSubscripts is undefined with both Metamask and WalletConnect providers. That's what I mean by unavailable. You can check that in the codesandbox I linked above

@cgewecke
Copy link
Collaborator

@Velenir

web3.currentProvider.supportsSubscripts is undefined with both Metamask and WalletConnect providers.

Ok interesting. It works as expected when connecting directly to a local client and have added E2E tests for that in #3165. I think the wallet/injected web3 case gets routed through this given provider logic. That's going to need a closer look.

Tbh, there's no community agreement about this interface atm.

@cgewecke cgewecke added the 1.x 1.0 related issues label Oct 27, 2019
@cgewecke cgewecke self-assigned this Oct 27, 2019
@Velenir
Copy link
Contributor Author

Velenir commented Oct 28, 2019

Almost in the given provider.
I've looked through the code and here when provider given to new Web3(provider) is a string, then an instance of HttpProvider | WebsocketProvider | IpcProvider is created, which implements supportsSubscriptions().
If not a string but a third-party provider (e.g. Metamask) is given, then provider is used as is whether it has supportsSubscriptions function or not.

An for version 2.0.0 third-party providers are wrapped in... different objects depending on provider, here

@nivida nivida added the Types Incorrect or missing types label Oct 28, 2019
@cgewecke
Copy link
Collaborator

If not a string but a third-party provider (e.g. Metamask) is given, then provider is used as is whether it has supportsSubscriptions function or not.

Nice! Thanks for finding that.

@nivida nivida mentioned this issue Nov 14, 2019
10 tasks
@cgewecke
Copy link
Collaborator

The types problem should be fixed on 1.2.4

The issue with supportsSubscription looks like it needs to wait for further development of EIP 1193 before it's improved.

Closing via #3162

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
1.x 1.0 related issues Types Incorrect or missing types
Projects
None yet
Development

No branches or pull requests

3 participants