Improve error message when giving the wrong password for a local token #4339

scristobal · 2025-03-13T17:47:51Z

Adds a new error variant Error::FailedToDecryptApiKey specifically to handle the case when the decryption of the Hex API token fails.

Hopefully, the changes will make the error message a bit more friendly and possibly more useful.

resolves #4317

lpil

Thank you!

It looks like the FailedToDecrypt is no longer used. Let's remove that error variant because of this.

lpil · 2025-03-20T12:24:56Z

compiler-cli/src/hex/auth.rs

-        let unencrypted = encryption::decrypt_with_passphrase(encrypted.as_bytes(), &password)?;
+        let unencrypted = encryption::decrypt_with_passphrase(encrypted.as_bytes(), &password)
+            .map_err(|e| Error::FailedToDecryptApiKey {
+                detail: e.to_string(),


Do not call to_string on our error type please.

I modified decrypt_with_passphrase to return a more generic DecryptError

gleam/compiler-core/src/encryption.rs

Line 29 in 578f878

) -> Result<String, DecryptError> {

Unfortunately some extra boilerplate is required as decrypt_with_passphrase can fail in two different ways

gleam/compiler-core/src/encryption.rs

Lines 15 to 24 in 578f878

// the function `decrypt_with_passphrase` has two possible failure cases:

// - when decryption fails

// - when the data was decrypted succesfully but the result is not UTF-8 valid

#[derive(Error, Debug)]

pub enum DecryptError {

#[error("unable to decrypt message: {0}")]

Decrypt(#[from] age::DecryptError),

#[error("decrypted message is not UTF-8 valid: {0}")]

Io(#[from] std::string::FromUtf8Error),

}

I would love to use thiserror's #[from] macro to automatically convert a DecryptError into a FailedToDecryptApiKey but it is not possible because of Eq and PartialEq derive macros here and those not being implemented by DecryptError variants...

Hence the manual map_err and the .to_string call.

An alternative will be to return directly a FailedToDecryptApiKey from decrypt_with_passphrase instead of a DecryptError.

That approach would be more direct and simple, but also would make decrypt_with_passphrase a bit less flexible.

I would be happy to refactor if you think simplicity is a better trade-off.

This looks great!! Thank you!!

lpil · 2025-03-20T12:25:53Z

compiler-core/src/error.rs

@@ -321,6 +321,9 @@ file_names.iter().map(|x| x.as_str()).join(", "))]

    #[error("Failed to decrypt data")]
    FailedToDecrypt { detail: String },
+
+    #[error("Failed to decrypt API key")]
+    FailedToDecryptApiKey { detail: String },


Could be any API key! FailedToDecryptLocalHexApiKey or such? The error attribute needs to say this info also.

modified both, the error variant name and the error attribute

gleam/compiler-core/src/error.rs

Lines 322 to 323 in 578f878

#[error("Failed to decrypt local Hex API key")]

FailedToDecryptLocalHexApiKey { detail: String },

lpil · 2025-03-20T12:26:59Z

compiler-core/src/error.rs

@@ -1487,6 +1490,21 @@ The error from the encryption library was:
                }]
            }

+            Error::FailedToDecryptApiKey { detail } => {
+                let text = wrap_format!("Unable to decrypt the local Hex token with the given password.


We call it an API key, not a token 🙏

modified token -> API key

gleam/compiler-core/src/error.rs

Line 1476 in 578f878

let text = wrap_format!("Unable to decrypt the local Hex API key with the given password.

lpil · 2025-03-20T12:27:05Z

compiler-core/src/error.rs

+    {detail}"
+);
+                vec![Diagnostic {
+                    title: "Failed to decrypt Hex token".into(),


modified Hex token -> local Hex API key

gleam/compiler-core/src/error.rs

Line 1482 in 578f878

title: "Failed to decrypt local Hex API key".into(),

scristobal · 2025-03-21T15:50:59Z

Thank you!

It looks like the FailedToDecrypt is no longer used. Let's remove that error variant because of this.

I removed that error variant and in exchange introduced DecryptError local to encryption:: decrypt_with_passphrase see my comment here

In a similar fashion, I also went out on a limb and replaced the generic encryption "data" error

gleam/compiler-core/src/error.rs

Lines 319 to 320 in fc5daa9

    
           #[error("Failed to encrypt data")] 
        
           FailedToEncrypt { detail: String },

with a narrower version, similar to FailedToDecryptLocalHexApiKey

gleam/compiler-core/src/error.rs

Lines 319 to 320 in 578f878

    
           #[error("Failed to encrypt local Hex APIT key")] 
        
           FailedToEncryptLocalHexApiKey { detail: String },

Let me know what do you think!

Thank you for your patience 🙏🏻

lpil · 2025-03-22T10:39:39Z

Is this ready for review? Press the button if so! :)

lpil

Wonderful! Thank you very much!

CHANGELOG.md

compiler-core/src/error.rs

lpil · 2025-03-22T19:46:14Z

compiler-cli/src/hex/auth.rs

-        let unencrypted = encryption::decrypt_with_passphrase(encrypted.as_bytes(), &password)?;
+        let unencrypted = encryption::decrypt_with_passphrase(encrypted.as_bytes(), &password)
+            .map_err(|e| Error::FailedToDecryptApiKey {
+                detail: e.to_string(),


This looks great!! Thank you!!

scristobal · 2025-03-23T06:59:17Z

@lpil unfortunately I accepted your suggestion before seeing your commit, that invalidated auto-merge. I tried to remove the commit by force pushing, but that didn't help either.

Anyway, hopefully everything is in order now.

lpil

Thanks again!

lpil reviewed Mar 20, 2025

View reviewed changes

lpil marked this pull request as draft March 20, 2025 12:29

scristobal force-pushed the issue-4317 branch from f6b0bdc to 15fd9ca Compare March 20, 2025 18:18

scristobal requested a review from lpil March 21, 2025 15:51

scristobal marked this pull request as ready for review March 22, 2025 12:27

scristobal added 2 commits March 22, 2025 19:06

improve error message when giving the wrong password for a local token

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

a68aea9

replace generic encrypt/decrypt data error messages

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

144ce7b

scristobal force-pushed the issue-4317 branch from 578f878 to 144ce7b Compare March 22, 2025 18:07

lpil reviewed Mar 22, 2025

View reviewed changes

Typos!

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

40f85c8

lpil enabled auto-merge (rebase) March 22, 2025 19:49

auto-merge was automatically disabled March 22, 2025 19:53
Head branch was pushed to by a user without write access

scristobal force-pushed the issue-4317 branch from 4fd8ac9 to 40f85c8 Compare March 22, 2025 19:57

typo (again)

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

c3fe88b

scristobal requested a review from lpil March 23, 2025 06:56

lpil approved these changes Mar 23, 2025

View reviewed changes

lpil merged commit 9bedc27 into gleam-lang:main Mar 23, 2025
12 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Improve error message when giving the wrong password for a local token #4339

Improve error message when giving the wrong password for a local token #4339

scristobal commented Mar 13, 2025

lpil left a comment

lpil Mar 20, 2025

scristobal Mar 21, 2025

lpil Mar 22, 2025

lpil Mar 20, 2025

scristobal Mar 21, 2025

lpil Mar 20, 2025

scristobal Mar 21, 2025

lpil Mar 20, 2025

scristobal Mar 21, 2025

scristobal commented Mar 21, 2025

lpil commented Mar 22, 2025

lpil left a comment

lpil Mar 22, 2025

scristobal commented Mar 23, 2025

lpil left a comment

	// the function `decrypt_with_passphrase` has two possible failure cases:
	// - when decryption fails
	// - when the data was decrypted succesfully but the result is not UTF-8 valid
	#[derive(Error, Debug)]
	pub enum DecryptError {
	#[error("unable to decrypt message: {0}")]
	Decrypt(#[from] age::DecryptError),
	#[error("decrypted message is not UTF-8 valid: {0}")]
	Io(#[from] std::string::FromUtf8Error),
	}

	#[error("Failed to decrypt local Hex API key")]
	FailedToDecryptLocalHexApiKey { detail: String },

Improve error message when giving the wrong password for a local token #4339

Improve error message when giving the wrong password for a local token #4339

Conversation

scristobal commented Mar 13, 2025

lpil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scristobal commented Mar 21, 2025

lpil commented Mar 22, 2025

lpil left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

scristobal commented Mar 23, 2025

lpil left a comment

Choose a reason for hiding this comment