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.

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

#[handle_result] should not rely on Result being referred to using specific tokens #852

Open
austinabell opened this issue Jun 24, 2022 · 10 comments
Labels

Comments

@austinabell
Copy link
Contributor

austinabell commented Jun 24, 2022

Currently, the attribute expects Result<T, E> specifically, which is a subset of syntax that this functionality allows.

The main cases that should be handled are:

Pulling a result type from another module

#[handle_result]
pub fn foo() -> ::core::result::Result<(), &'static str> {...}

or a type alias to result:

type Result<T> = core::result::Result<T, &'static str>;

#[handle_result]
pub fn foo() -> Result<()> {...}

Where having both of these would allow a common pattern like:

#[handle_result]
pub fn foo() -> anyhow::Result<()> {...}

Motivation: near.zulipchat.com/#narrow/stream/300659-Rust-.F0.9F.A6.80/topic/anyhow.3F/near/287320608

@uint
Copy link
Contributor

uint commented Jul 20, 2023

At macro expansion time, the compiler operates on tokens and has no access to type information. That makes this whole deal rather hacky.

I'm toying with the idea of moving "Ok type" extraction to the type/trait system instead. I suspect that will work out much better.

@uint uint self-assigned this Jul 20, 2023
@uint uint changed the title Allow more result patterns with #[handle_result] #[handle_result] should not rely on Result being referred to using specific tokens Jul 20, 2023
@uint
Copy link
Contributor

uint commented Jul 20, 2023

Resolving this ticket isn't enough to support anyhow. Opened another issue: #1055

@frol
Copy link
Collaborator

frol commented Jul 20, 2023

I'm toying with the idea of moving "Ok type" extraction to the type/trait system instead. I suspect that will work out much better.

This is exactly the direction I would love to see it going. Thanks for looking into that direction.

@miraclx
Copy link
Contributor

miraclx commented Apr 2, 2024

Did exactly this last night for Calimero, figured I'd mention it here as well.

The idea is to inject scaffolding for any return type, and specialize for results using the trait domain. Rust treats direct association with a higher precedence than trait-enabled ones. It's the same magic that powers https://github.com/nvzqz/impls.

Once implemented, you'll no longer need the annotation, nor will you be forcing downstream users to return a Result.

Method Signature IntoResult Representation
fn() Result<(), Infallible>
fn() -> T Result<T, Infallible>
fn() -> Result<T, E> Result<T, E>
fn() -> anyhow::Result<T> Result<T, anyhow::Report>

At which point, you can do with the error whatever thou wilt.

The foundation of it (can be re-exported from near_sdk::__private)

struct WrappedReturn<T>(pub T);

impl<T, E> WrappedReturn<Result<T, E>> {
    fn into_result(self) -> Result<T, E> {
        let WrappedReturn(result) = self;
        result
    }
}

trait IntoResult {
    type Ok;
    type Err;

    fn into_result(self) -> Result<Self::Ok, Self::Err>;
}

impl<T> IntoResult for WrappedReturn<T> {
    type Ok = T;
    type Err = std::convert::Infallible;

    fn into_result(self) -> Result<Self::Ok, Self::Err> {
        let WrappedReturn(value) = self;
        Ok(value)
    }
}

then in the codegen (same as usual, with a little tweak)

let result = contract.method();

#[allow(unused_imports)]
use near_sdk::__private::IntoResult;
match near_sdk::__private::WrappedReturn(result).into_result() {
    Ok(result) => {
        let result = serde_json::to_vec(&result)
            .expect("Failed to serialize the return value using JSON.");
        near_sdk::env::value_return(&result);
    }
    Err(err) => near_sdk::FunctionError::panic(&err),
}

You might want to use a custom Infallible type with dummy impls of Serialize if you go with structured errors or at least AsRef<str> for FunctionError::panic.

Ah, just saw @uint alluded to this sometime back, noice - #749 (comment)

@frol frol added the ODHack label Aug 1, 2024
@frol frol unassigned uint Aug 1, 2024
@faytey
Copy link

faytey commented Aug 2, 2024

I would like to hop on this @frol
To tackle this issue I'll need to enhance the procedural macro to accommodate different Result types/forms, including those from other modules or using type aliases. Also, I'd add the dependencies to the cargo.toml file, then check the Handle_result attribute works with all forms of Result, I'd also closely work with the maintainer if I face any blockers

@frol
Copy link
Collaborator

frol commented Aug 3, 2024

@faytey Sure, please, go for it. Maybe you need to apply through ODHack website, so I will wait until that before I assign you to the issue, but feel free to start hacking!

Copy link

onlydustapp bot commented Aug 3, 2024

Hi @frol!
Maintainers during the ODHack #6.0 will be tracking applications via OnlyDust.
Therefore, in order for you to have a chance at being assigned to this issue, please apply directly here, or else your application may not be considered.

@faytey
Copy link

faytey commented Aug 3, 2024

I am applying to this issue via OnlyDust platform.

My background and how it can be leveraged

I have experience with react/nextjs, typescript, javascript, solidity, cairo, rust.

How I plan on tackling this issue

To tackle this issue I'll need to enhance the procedural macro to accommodate different Result types/forms, including those from other modules or using type aliases. Also, I'd add the dependencies to the cargo.toml file, then check the Handle_result attribute works with all forms of Result, I'd also closely work with the maintainer if I face any blockers

@frol
Copy link
Collaborator

frol commented Aug 3, 2024

@faytey this issue requires deep understanding of type system and fairly advanced Rust experience, I don't think you will succeed here

@faytey
Copy link

faytey commented Aug 3, 2024

@faytey this issue requires deep understanding of type system and fairly advanced Rust experience, I don't think you will succeed here

Alright

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: NEW❗
5 participants