Skip to content

fix(python): correct CLI error message on uninitialized device #5613

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

Merged
merged 1 commit into from
Aug 22, 2025

Conversation

romanz
Copy link
Contributor

@romanz romanz commented Aug 21, 2025

[no changelog]

@romanz romanz self-assigned this Aug 21, 2025
@trezor-bot trezor-bot bot added this to Firmware Aug 21, 2025
@github-project-automation github-project-automation bot moved this to 🔎 Needs review in Firmware Aug 21, 2025
Copy link

github-actions bot commented Aug 21, 2025

en main(all)

model device_test click_test persistence_test
T2T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3B1 test(all) main(all) test(all) main(all) test(all) main(all)
T3T1 test(all) main(all) test(all) main(all) test(all) main(all)
T3W1 test(all) main(all) test(all) main(all) test(all) main(all)

Latest CI run: 17149560160

@romanz romanz marked this pull request as ready for review August 22, 2025 06:37
@romanz romanz requested a review from matejcik as a code owner August 22, 2025 06:37
@romanz romanz requested a review from ibz August 22, 2025 06:37
@romanz romanz merged commit b4f857b into main Aug 22, 2025
103 checks passed
@romanz romanz deleted the romanz/fix-error-message branch August 22, 2025 06:53
@github-project-automation github-project-automation bot moved this from 🔎 Needs review to 🤝 Needs QA in Firmware Aug 22, 2025
@romanz romanz added the trezorlib Python library and the command line trezorctl tool. label Aug 22, 2025
@matejcik
Copy link
Contributor

we already have Failure(NotInitialized)
the exception name is horrible and these two should coerce to the same thing

please call the exception NotInitializedError, make sure it returns "Device is not initialized." as __str__(), and in session.call() make sure that FailureType.NotInitialized is raised as this type (similar to what Cancelled is doing)

@@ -307,6 +307,9 @@ def session_context(
sys.exit(1)
except exceptions.FailedSessionResumption:
sys.exit(1)
except exceptions.DerivationOnUninitaizedDeviceError:
click.echo("Device is not initialized.")
sys.exit(1)
except Exception:
click.echo("Failed to find a Trezor device.")
Copy link
Contributor

@matejcik matejcik Aug 22, 2025

Choose a reason for hiding this comment

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

we definitely want to emit the exception in the log
maybe it's also a good idea to say f"(error was: {e})" or something?

romanz added a commit that referenced this pull request Aug 24, 2025
romanz added a commit that referenced this pull request Aug 24, 2025
romanz added a commit that referenced this pull request Aug 24, 2025
romanz added a commit that referenced this pull request Aug 24, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
trezorlib Python library and the command line trezorctl tool.
Projects
Status: 🤝 Needs QA
Development

Successfully merging this pull request may close these issues.

3 participants