Remove console.error

[HBashanaE]

Remove console.error on 429, not to log error on client side even if the errors are handled.

As per @buffalojoec 's suggestion, parameterized the log.

[changeset-bot]

⚠️ No Changeset found

Latest commit: a7092df

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

[rangatechnologies]

Remove console.error on 429, not to log error on client side even if the errors are handled.

Yes i am also facing with same issue, this is fundamental. Please remove this.

[buffalojoec]

I think there's maybe a better way to do this. Some developers may actually like and/or depend on this message in their app somehow.

What about plumbing a config through Connection that can allow you to disable this log message (and likely others)? Something like noLog: true, maybe?

[HBashanaE]

@buffalojoec Make sense. Let's do like that.

[buffalojoec]

Cool! But you can't actually access that parameter. This function is private and needs to be invoked by the Connection class.

solana-web3.js/packages/library-legacy/src/connection.ts

Lines 3146 to 3153 in d3d5399

	this._rpcClient = createRpcClient(
	endpoint,
	httpHeaders,
	fetch,
	fetchMiddleware,
	disableRetryOnRateLimit,
	httpAgent,
	);

[buffalojoec]

Great, thanks! I think if we're going to add this parameter, let's use it everywhere else there's a console output someone might want to disable. There's a handful more around the Connection class that can use this feature.

[steveluscher]

At this point, I think I'm unlikely to accept this change unless you can explain more about the problem.

Yes i am also facing with same issue, this is fundamental.

Other than the log being annoying, is there any other issue? I'm looking for something that affects the operation of your app.

Console logs were never the right thing to do here, but we're here now, and I see only downsides to making a change now.

• This will break any developers who are spying on console.error and depend on the ability to scrutinize this log
• Adding config is always bad
  • every bit of config is cognitive overhead for the developer (log what? log to where? log, as opposed to what?)
  • when the config is switched off you still bundle the code – now dead code – controlled by that config. The right way to do config like this is by replacing constants, statically, at build time, so that the build system can reap the dead branches of code.

[steveluscher]

A parameter has not been added to the top-level Connection constructor for years; doing so should be done with care, and only in cases where the parameter is fundamental to the nature of the connection instance itself. I don't think this parameter meets that bar.

Also, at this high a level, it won't be obvious to developers what log controls. There are tons of logs in this class, and this only controls one of them, without actually describing that target in the name of the argument.

Related: functions with lists of boolean arguments yield bad outcomes. Read more here.

If this belonged here at all, it belongs on the ConnectionConfig type.

[steveluscher]

I'm going to close this PR at this time; if you have time to revisit this and answer the question above, we can pick it up again!

[mergify]

⚠️ The sha of the head commit of this PR conflicts with #2681. Mergify cannot evaluate rules on this PR. ⚠️