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

Producing Errors for JS in Class Constructor #141

Open
NovaLogicDev opened this issue Mar 23, 2021 · 7 comments
Open

Producing Errors for JS in Class Constructor #141

NovaLogicDev opened this issue Mar 23, 2021 · 7 comments
Assignees
Labels

Comments

@NovaLogicDev
Copy link

trivially

struct MyClass {
  a_string: String
}

#[node_bindgen]
impl MyClass {
  #[node_bindgen(constructor)]
  fn new() -> Self {
    Self {
      a_string: function_that_returns_result().unwrap()
    }
  }
}

This example causes the JS application to crash completely if the function returns an error, which would be great if that's what I wanted, but instead, I would like to be able to have the constructor error out, producing an error to be caught by JS instead of returning Self.
Now I'm newish to rust so maybe I missed something but the only way I understand to get an error up the call stack is to keep returning Results, however #[node_bindgen(constructor)] only accepts a return of "Self."
Understandably it could be possible to make a_string type Result, however, I would have to check it in every function and that simply causes problems for certain more complex objects that I intend to use.

So I am here to ask, how do I throw errors for catching in JS from my struct "constructor"?

@sehz sehz added the triage label Mar 23, 2021
@sehz sehz self-assigned this Mar 24, 2021
@sehz
Copy link
Collaborator

sehz commented Mar 25, 2021

There are 2 approaches I can think of:
First approach is have manually call N-API to create error like below:

struct ClassWithError {
    val: i32
  }
  
#[node_bindgen]
impl ClassWithError {
    #[node_bindgen(constructor)]
    fn new(env: node_bindgen::core::val::JsEnv, val_str: String) -> Self {

        match val_str.parse::<i32>() {
            Ok(val) => {
                Self {
                    val: 22
                }
            },
            Err(err) => {
                env.create_error(&format!("parse error: {}",err));
                Self {
                    val: 0
                }
            }
        }
        
    }
}

However, we need to fix procedure macro to accept JsEnv like normal method.

Second approach is allow constructor to return Result and have procedure macro to generate Error conversion.

struct ClassWithError {
    val: i32
  }
  
#[node_bindgen]
impl ClassWithError {
    #[node_bindgen(constructor)]
    fn new(val_str: String) -> Result<Self,std::num::ParseIntError> {           
                OK(Self {
                    val: val_str.parse()?
                })
     }
   
}

@NovaLogicDev
Copy link
Author

So the second option seems like the better of the two in my case. This is because it seems that in the event of an error, the construction would produce the Error result instead of an instance with some arbitrary "null-like" value, which wouldn't be clean to implement in a case where you had some, say complex object in the struct like say a File or something.

So then my further question because I'm not exactly sure what you mean, what would the procedure macro to "generate Error conversion" look like? what would its objective be? And what would be a good resource for me to understand how that would be done, like I said I'm somewhat new at rust?

@mkawalec
Copy link
Contributor

I think #155 simplifies the issues @NovaLogicDev has had - now structs and enums to-js conversions can be autoderived. Also structured errors can be returned from Result, so something like

#[node_bindgen]
enum MyErrorType {
  Constructor,
  SomethingElse
}

#[node_bindgen]
impl ClassWithError {
    #[node_bindgen(constructor)]
    fn new(val_str: String) -> Result<Self, MyErrorType> {           
                Err(MyErrorType::Constructor)
     }
   
}

should work and throw "Constructor" once v5 is merged and released.

@marcmo
Copy link
Contributor

marcmo commented Mar 9, 2023

@sehz is this supposed to work now with v5? I mean is a constructor supported that returns a Result<Self, ...>?

@sehz
Copy link
Collaborator

sehz commented Mar 21, 2023

@marcmo current release is 5.1.0. So should work. please check

@dignifiedquire
Copy link

This doesn't work on the latest release

error[E0308]: mismatched types
   --> src/node.rs:199:1
    |
199 | #[node_bindgen]
    | ^^^^^^^^^^^^^^^ expected `IrohNode`, found `Result<IrohNode, IrohError>`
    |
    = note: expected struct `node::IrohNode`
                 found enum `Result<node::IrohNode, IrohError>`

with the constructor being defined as

#[node_bindgen]
impl IrohNode {
    #[node_bindgen(constructor)]
    pub fn new_js() -> Result<Self, Error> {
       // .. actual logic
    }
}

@sehz sehz assigned ozgrakkurt and unassigned sehz Aug 30, 2023
@sehz
Copy link
Collaborator

sehz commented Aug 30, 2023

@ozgrakkurt triage

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants