Make SET_FLAG_IF more paranoid

[hoodmane]

This fixes a regression introduced in #3283. To be properly paranoid we should assume that each property access can raise an error.

The bug also depends on this branch here:
https://github.com/pyodide/pyodide/blob/main/src/core/python2js.c#L833-L834
which is a bit suspect...

[ryanking13]

Thanks!

[ryanking13]

Shouldn't we propagate the error, or maybe at least show a warning message? If we ignore the error, then the object will have invalid type flags I think.

[hoodmane]

We can't throw an error here because then the JavaScript object becomes completely inaccessible from Python (as previously was the case with objects produced viaObject.create(null)). It's possible to work around missing type flags via slightly non idiomatic code. It's not possible to work around "touching this object in any way throws an error".

But note also that this is extremely rare. Basically the main thing we are concerned about is stuff like:

py_func({ get length() { throw new Error(); } });

It's actually pretty funny that hypothesis managed to find a code path that is equivalent to this.

But you are right that ideally this should produce a JsProxy such that len(x) throws a JsException, not one that throws TypeError: object of type 'type' has no len(). Particularly for extremely pathological stuff like:

py_func({ 
    get length() { 
        if(firstTimeCalled){
            throw new Error(); 
        }
        return 7;
    } 
});

To get something that behaves correctly here we should do:

function hasProperty(obj, prop) {
  while (obj) {
    if (Object.getOwnPropertyDescriptor(obj, prop)) {
      return true;
    }
    obj = Object.getPrototypeOf(obj);
  }
  return false;
}

I think we should still catch and throw away errors, but we should set fail_test to true and when compiled in debug mode we should log them.

[hoodmane]

Well currently we have:

$ len(run_js("({ get length() { return 'a'; } })"))
TypeError: object of type 'pyodide.ffi.JsProxy' has no len()

But if we changed it to not execute the getter, we would get:

$ len(run_js("({ get length() { return 'a'; } })"))
TypeError: 'str' object cannot be interpreted as an integer

which is I guess fine. But then what about:

len(run_js("({ length: 'a' })"))

or:

len(run_js("({ length: undefined })"))

I guess in Python you can say:

class A:
   def __len__(self):
      return None

and the expected behavior is that len(A()) raises

TypeError: 'NoneType' object cannot be interpreted as an integer

[hoodmane]

But I think the most important thing is that JsProxy_create should never fail.