Argument count mismatch UX friction

[wesgarland]

Issue type

Bug

How did you install PythonMonkey?

None

OS platform and distribution

No response

Python version (python --version)

No response

PythonMonkey version (pip show pythonmonkey)

No response

Bug Description

In JS, it's pretty normal to have variadic functions, or to simply not bother using some of the arguments for a function.

I bumped into a weird bit of UX friction here

Error loading bundle: Error in file [python code], on line 0:
Error: Python str: go.<locals>.<lambda>() takes 1 positional argument but 2 were given
Stack Trace: 
  emit@webpack://dcp/./node_modules/events/events.js?:153:17
  concurrency$$Synchronizer$set@webpack://dcp/./src/common/concurrency.js?:96:8
  Connection530$$connect@webpack://dcp/./src/protocol/connection.js?:656:16

the code I wrote looks like this

    conn.state.on("change", lambda state: print("connection now", state))

What's going on here is that the JS code invoked the event handler with two arguments, but I only care about the first. This wouldn't be an error in JS, but it is in python.

Is this little bit of friction solvable? Like some way to invoke a wrapped python function from JS where it just doesn't care that it didn't use all of the possible arguments?

Standalone code to reproduce the issue

No response

Relevant log output or backtrace

No response

Additional info if applicable

No response

What branch of PythonMonkey were you developing on? (If applicable)

No response

[zollqir]

This is an annoying difference between python and JS.

Conceivably, when we call a python function from JS, we could check the number of arguments the python function takes and ignore any past that number that were passed to the JS wrapper, but then that would violate the principle of least surprise since we would be silently dropping args that the user may assume would cause an exception.

Variadic functions do exist in python, but a function needs to explicitly be declared as a variadic function (like so: def func(*args):), unlike in JS where effectively any function is variadic thanks to the arguments object.

[wesgarland]

@caleb-distributive I'm not sure about the principle of least surprise here

If the JS interface is callback(a,b,c) and a JS dev only needs argument a, it's pretty common to only write the function callback(a). In fact, there are a lot of functions out there in JS-land which have extra arguments for debugging that they don't even document because they're not part of the API. Most commonly seen in event handler implementations. Often a handle to the thing that triggered the event is passed as an extra argument beyond the actually event handler's formal API.

So a JS dev writing function callback(a) that would work fine in JS might very well be surprised that the same function written in Python would throw because JS-land passed it "too many arguments"

[philippedistributive]

@wesgarland do we care about the principle of least-surprise in this context, or shall we proceed with the wrapper logic? @caleb-distributive what kind of args can be assume to cause an exception? Sounds like a pretty detailed contract? How was it provided? Most of the time the method implementation would not conform to a protocol saying it must throw in such and such contextes

[wesgarland]

So, to keep thinking about this problem, I wrote a bug the other day and finally went looking for it this morning

connection(i:1): shutting down.
Exception in callback None()
handle: <TimerHandle cancelled when=489872.024765606>
Traceback (most recent call last):
  File "/usr/lib/python3.10/asyncio/events.py", line 80, in _run
    self._context.run(self._callback, *self._args)
pythonmonkey.SpiderMonkeyError: Error in file [python code], on line 0, column 0:
Error: Python str: go.<locals>.<lambda>() takes 0 positional arguments but 1 was given
Stack Trace: 
  emit@webpack://dcp/./node_modules/events/events.js?:153:17
  #finalize@webpack://dcp/./src/protocol-v4/connection/connection.js?:1434:10
  finishShutdown@webpack://dcp/./src/protocol-v4/connection/connection.js?:1596:23

The JS code at connection.js:1434 is

this.emit('close', rejectError);

The Python code I wrote that triggered this was

conn.on("close", lambda: print("connection closed"))

and the fix is

conn.on("close", lambda unused: print("connection closed"))

So the friction point is that as a JS developer, I'm expecting that first callback to work. It would work perfectly it if it was a JS function. Callbacks that crash are actually tricky to debug and tricky to test in many cases, because the code can be hard to reach.

If it's feasible to patch this up - what is the cost? Should we? I'm still thinking about that.

Effectively, the JS function calling protocol is always variadic, with the opportunity to name some of the argument positions.

[wesgarland]

Oh, and that bug from this morning is especially nasty. The argument to the close event implemented in dcp5.2 with error object is not present in dcp5.3. It's not part of the official API, this is one of those "extra debugging arguments" I was talking about in the top of the PR.

[philippedistributive]

Let's do this, let's do the wrapper thing!