Skip to content

Conversation

stevenj
Copy link
Collaborator

@stevenj stevenj commented Aug 7, 2025

Description

In line with the move to try and remove Debug prints from anywhere but Unit Tests, and Debug builds, this PR removes
all but the Debug print lints from the wasm bindings.

There is currently an issue in clippy which prevents the clippy lint for these issues being silenced in a macro like bindgen! which
is why they have not silenced. There are also no bindgen! options to not emit Debug implementations of automatically created types.

During this process there were a couple of minor changes which helped the process.

  1. Any Specificially types Error, which was not used as a Specifically typed error, was changed to anyhow! this reduces the maintenance burden created by Typed errors, where no logic ever turns on the Error type itself.
  2. Mutex<Arc<HashMap<>>> in the http connection manager was converted to a Dashmap. This made dealing with that type both more ergonomic, and also elimated a handful of error cases, which no longer needed errors thrown for the,
  3. All unit tests were made so they will ONLY be able to run in debug builds.
  4. Only Structs that are required to have Debug implemented to satisfy assert_* macros have been derived, and only in debug builds.
  5. All Debug formatting has been eliminated.
  6. Anyhow errors are serialized to error! logs using ToString which is equivalent to Display and just extracts the string in the anyhow! error, which is controlled and can not leak unexpected data. (Another reason to use Anyhow).
  7. Silences lints on all todo! because this is not production code. But this will need to be reverted before we are production ready.

Related Issue(s)

No Specific Issue Yet

Breaking Changes

None, everything that was changed should be transparent.

Related Pull Requests

This was tested and built against this PR: input-output-hk/catalyst-ci#423
But that version is not used in this PR, because it is not yet ready to be merged, pending further testing.
Once merged, all these changes will eliminate a huge source of errors it will cause otherwise.

Please confirm the following checks

  • My code follows the style guidelines of this project
  • I have performed a self-review of my code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream module

@stevenj stevenj self-assigned this Aug 8, 2025
@stevenj stevenj added the review me PR is ready for review label Aug 8, 2025
@stevenj stevenj added this to Catalyst Aug 8, 2025
@stevenj stevenj added rust Pull requests that update Rust code squad: hermetics Hermes Backend, System Development & Integration Team p: normal The issue or pull request is of normal priority and can be addressed in due course fyi: hermetics For information - Hermes Backend, System Development & Integration Team rr: hermetics Required review by hermes team refactor Code refactoring task labels Aug 8, 2025
@stevenj stevenj moved this from New to 👀 In review in Catalyst Aug 8, 2025
cong-or
cong-or previously approved these changes Aug 12, 2025
Copy link
Contributor

@cong-or cong-or left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks good, I would agree with the comments from Rafal if you get a chance.

@stevenj stevenj enabled auto-merge (squash) August 27, 2025 07:49
@stevenj stevenj disabled auto-merge August 27, 2025 07:50
@stevenj stevenj merged commit 3d30615 into main Aug 27, 2025
43 checks passed
@stevenj stevenj deleted the fix/reduce-use-of-Debug-print branch August 27, 2025 07:50
@github-project-automation github-project-automation bot moved this from 👀 In review to ✅ Done in Catalyst Aug 27, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
fyi: hermetics For information - Hermes Backend, System Development & Integration Team p: normal The issue or pull request is of normal priority and can be addressed in due course refactor Code refactoring task review me PR is ready for review rr: hermetics Required review by hermes team rust Pull requests that update Rust code squad: hermetics Hermes Backend, System Development & Integration Team
Projects
Status: ✅ Done
Development

Successfully merging this pull request may close these issues.

4 participants