Skip to content

*js.Error and *js.Object treating null objects differently #275

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

Open
flimzy opened this issue Aug 22, 2015 · 8 comments
Open

*js.Error and *js.Object treating null objects differently #275

flimzy opened this issue Aug 22, 2015 · 8 comments

Comments

@flimzy
Copy link
Member

flimzy commented Aug 22, 2015

I originally asked about this on the Google Group, but GH seems to be more active, so I'm re-posting here (plus markdown!!)

I'm not sure if I've run into a bug, or if I'm misusing GopherJS, but I've run into a problem with the way my Go code receives errors passed by Javascript Callbacks. To demonstrate the problem, I've built this code:

My main.js:

window.foo = {
    nothing: function(cb) {
        return cb(null);
    }
}
require('main');

And main.go:

package main

import (
    "github.com/gopherjs/gopherjs/js"
    "honnef.co/go/js/console"
)

func main() {
    result := nothing()
    console.Log(result)
    if result == nil {
        console.Log("result is nil")
    }
    if result != nil {
        console.Log("result is not nil")
    }
}

func nothing() *js.Error {
    c := make(chan *js.Error)
    go func() {
        js.Global.Get("foo").Call("nothing",func(err *js.Error) {
            c <- err
        })
    }()
    return <-c
}

The output on my javascript console (in Chrome 43, FWIW) is:

null
result is not nil

If I do a s/*js.Error/*js.Object/ in main.go, I get the expected result:

null
result is nil

So it seems that somehow the Error struct around *js.Object is obfuscating the nullness of the javascript object? Am I doing this wrong?

@dmitshur
Copy link
Member

This sounds related to #240. Before that PR, there was no clean mapping from js.Undefined to JavaScript's undefined either.

Perhaps this is yet another similar unhanded case. I need to think about it more.

@dmitshur
Copy link
Member

I've honestly almost never dealt with js.Errors, so I can't speak from experience here

js.Error is defined as:

type Error struct {
    *Object
}

I tried your example code and I've noticed if I add these logs:

if result.Object == nil {
    console.Log("result.Object is nil")
}
if result.Object != nil {
    console.Log("result.Object is not nil")
}

Then the output is as expected:

result.Object is nil

Perhaps that's the right way to check if the error was nil? I'm actually not quite sure why a wrapper struct is needed at all. Perhaps it's to enable the Stack method?

Another possibility could've been to expose that via an interface that js errors implement, ala:

var err error
err = foo()
if jserr, ok := err.(js.Error); ok {
    fmt.Println(jserr.Stack())
}

Anyway, I don't think I can comment beyond this, you'll want to wait and see what @neelance says.

@dmitshur
Copy link
Member

Actually, looking at this one piece of code that deals with possible *js.Errors, I'm not sure if that's correct:

https://github.com/gopherjs/websocket/blob/867cdcb6edcfa907c935102a8b91c3d85317747c/wrapper.go#L106-L114

Since it only checks if jsErr is nil and not jsErr.Object.

Edit: Oh, never mind. It doesn't necessarily do something wrong. It'd be the user's responsibility to look at jsErr.Object, presumably.

dmitshur added a commit to shurcooL/play that referenced this issue Aug 24, 2015
@flimzy
Copy link
Member Author

flimzy commented Aug 24, 2015

I don't know about all the internal details. I just know that an object that conforms to the error interface, but can't be checked with != nil like normal errors, is anything but expected :)

If you need my help to dig into the issue a bit, let me know. I'm not afraid of getting my hands dirty, but I really know very little about the internals at play here. But if I need to familiarize myself, I can.

@neelance
Copy link
Member

The intention of *js.Error is just to encapsulate a JS error when it is "catched" via Go's recover. It was not meant to be passed to/from JS functions, but it's still possible because of its type signature. However, we may want to reconsider what happens when null gets internalized to such a pointer to a struct. @shurcooL @dominikh Do you think it makes sense to internalize to nil instead of synthesizing a struct and then put nil into the *js.Object field?

neelance added a commit that referenced this issue Oct 18, 2015
@tj
Copy link

tj commented May 7, 2017

Doing if err.Object != nil { is definitely unintuitive, I was trying *js.Error != nil as well. I think part of it is a documentation issue, I didn't even see anything about js.Error or how to handle JS-land callbacks w/ errors etc.

@dmitshur
Copy link
Member

dmitshur commented May 7, 2017

However, we may want to reconsider what happens when null gets internalized to such a pointer to a struct. @shurcooL @dominikh Do you think it makes sense to internalize to nil instead of synthesizing a struct and then put nil into the *js.Object field?

That makes sense to me. Sorry, didn't see this question earlier.

@myitcv
Copy link
Member

myitcv commented May 8, 2017

Maybe we try and fix this as part of #617?

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

No branches or pull requests

6 participants