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.

Sign up for GitHub

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

[Refactor] Handling exceptions on execute raw #53

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
+73 −13
Open

[Refactor] Handling exceptions on execute raw #53

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
49fb393
Added handling exceptions on execute raw
cristianFleita Jun 28, 2024
bd60588
Merge branch 'main' into handle-execute-raw-exceptions
cristianFleita Jul 18, 2024
cf41a08
Merge branch 'main' into handle-execute-raw-exceptions
cristianFleita Jul 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
52 changes: 43 additions & 9 deletions Assets/Dojo/Plugins/WebGL/starknet.jslib
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -41,15 +41,49 @@ mergeInto(LibraryManager.library, {
account.setBlockId(UTF8ToString(blockId));
},
AccountExecuteRaw: async function (accountPtr, callsStr, cb) {
const account = wasm_bindgen.Account.__wrap(accountPtr);
const calls = JSON.parse(UTF8ToString(callsStr));
const txHash = await account.executeRaw(calls);
const bufferSize = lengthBytesUTF8(txHash) + 1;
const buffer = _malloc(bufferSize);
stringToUTF8(txHash, buffer, bufferSize);

account.__destroy_into_raw();
dynCall_vi(cb, buffer);
let account;
let buffer;

try {
account = wasm_bindgen.Account.__wrap(accountPtr);
const calls = JSON.parse(UTF8ToString(callsStr));

// Execute the raw calls and get the transaction hash
const txHash = await account.executeRaw(calls);

// Create a success message
const message = JSON.stringify({ success: true, result: txHash });

// Allocate buffer for the message
const bufferSize = lengthBytesUTF8(message) + 1;
buffer = _malloc(bufferSize);
stringToUTF8(message, buffer, bufferSize);

// Call the callback function with the success message
dynCall_vi(cb, buffer);
} catch (error) {
// Log the error
console.error('Starknet call error:', error);

// Create an error message
const message = JSON.stringify({ success: false, error: error.toString() });

// Allocate buffer for the error message
const bufferSize = lengthBytesUTF8(message) + 1;
buffer = _malloc(bufferSize);
stringToUTF8(message, buffer, bufferSize);

// Call the callback function with the error message
dynCall_vi(cb, buffer);
} finally {
// Clean up the account object
if (account) {
account.__destroy_into_raw();
}
if (buffer) {
_free(buffer);
}
}
},
AccountDeployBurner: async function (accountPtr, privateKey, cb) {
const account = wasm_bindgen.Account.__wrap(accountPtr);
Expand Down
13 changes: 10 additions & 3 deletions Assets/Dojo/Runtime/Starknet/Account.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,16 @@ private unsafe Account(dojo.Account* account, SigningKey signingKey)
// webgl js interop starknet bindings
public async Task<FieldElement> ExecuteRaw(dojo.Call[] calls)
{
var res = await StarknetInterop.AccountExecuteRawAsync(await account.Task, calls);

return res;
try
{
var res = await StarknetInterop.AccountExecuteRawAsync(await account.Task, calls);
return res;
}
catch (Exception e)
{
Debug.LogError($"Error in ExecuteRaw: {e.Message}");
Copy link
Collaborator

Choose a reason for hiding this comment

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

I feel like if we end up wrapping the error by a try on the JS side and returning a Result then we shouldn't even throw. Since we wouldn't necessarily want to stop the game loop just because a transaction failed. We could maybe make ExecuteRaw return if the transaction failed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Oh yes, you got a point.

I could wrap the result of the execution on this class:

public class Result<T>
{
        public bool IsSuccess { get; }
        public T Value { get; }
        public string Error { get; }

        private Result(T value, string error, bool isSuccess)
        {
            Value = value;
            Error = error;
            IsSuccess = isSuccess;
        }

        public static Result<T> Success(T value) => new Result<T>(value, null, true);
        public static Result<T> Failure(string error) => new Result<T>(default(T), error, false);
 }

What do you think?

throw;
}
}
#else
private unsafe FieldElement ExecuteRawSync(dojo.Call[] calls)
Expand Down
21 changes: 20 additions & 1 deletion Assets/Dojo/Runtime/Starknet/StarknetInterop.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,10 +54,29 @@ public class AccountExecuteRawHelper
[MonoPInvokeCallback(typeof(Action<string>))]
public static void Callback(string result)
{
Tcs.SetResult(new FieldElement(result));
try
{
var message = JsonConvert.DeserializeObject<CallbackMessage>(result);

if (message.Success)
Tcs.SetResult(new FieldElement(message.Result));
else
Tcs.SetException(new Exception(message.Error));
}
catch (Exception ex)
{
Tcs.SetException(ex);
}
}
}

private class CallbackMessage
{
public bool Success { get; set; }
public string Result { get; set; }
public string Error { get; set; }
}

struct SerializedCall
{
public SerializedCall(string to, string selector, dojo.FieldElement[] calldata)
Expand Down