'TypeError: Attempted to wrap undefined property as function' while stubbing a module function

[mykola-mokhnach]

Describe the bug We are facing a strange issue in https://github.com/appium/appium/pull/18844

There we are trying to stub the getAndroidBinaryPath appium-adb module function, but getting the TypeError: Attempted to wrap undefined property getAndroidBinaryPath as function error: https://github.com/appium/appium/actions/runs/5752032818/job/15592089892?pr=18844

The error itself is cased by unit spec https://github.com/appium/appium/blob/9a5c9ae55c97ea3e5c604ecbc3f972480d091ba9/packages/doctor/test/unit/android.spec.js#L62

To Reproduce Steps to reproduce the behavior:

1. Checkout the the branch from the above PR
2. cd appium && npm i && cd packages/doctor
3. npm run test

Expected behavior Unit tests should pass

Screenshots If applicable, add screenshots to help explain your problem.

Context (please complete the following information):

• Library version: please verify that the bug exists in the latest release: 15.1.2
• Environment: Ubuntu
• Example URL:
• Other libraries you are using:

Additional context I have also tried to stub the function call like below, although it also did not work (no error was thrown, but the stub just has not been called properly while invoking the original code):

import {createSandbox} from 'sinon';
import * as adb from 'appium-adb';

...  
const sandbox = createSnadbox();
const stub = sandbox.stub(adb, 'getAndroidBinaryPath').resolves('yolo');
....
// calling the original method here does still invoke the original function instead of the stubbed one

[fatso83]

The reproduction case fails when installing npm i:

carlerik at Carl-Eriks-MacBook-Air in ~/dev/appium ((9a5c9ae...))
$ npm i
npm ERR! code ERR_SOCKET_TIMEOUT
npm ERR! network Socket timeout
npm ERR! network This is a problem related to network connectivity.

I get this consistently. Only affects this problem. Could not see why from the debug logs.

I did another repro for your other attempt:

npm init -y
npm i sinon
npm i appium-adb
vim package.json # set "type": "module"
pbpaste > index.js

That will unsurprisingly throw TypeError: ES Modules cannot be stubbed. Using console.dir(adb) also show getAndroidBinaryPath to be present:

  __esModule: true,
  default: {
    DEFAULT_OPTS: [Getter],
    ADB: [Getter],
    DEFAULT_ADB_PORT: [Getter],
    getAndroidBinaryPath: [Getter],
    getSdkRootFromEnv: [Getter],
    default: [class ADB]
  },
  getAndroidBinaryPath: [AsyncFunction: getAndroidBinaryPath],
  getSdkRootFromEnv: [Function: getSdkRootFromEnv]
}

Seeing that you are not getting this error, I guess you have a transpilation step that compiles it to CJS. A native ES Module is a immutable namespace, meaning you cannot modify the exports. When transpiling this to Common JS most bundlers/transpilers emulate this by creating an object where the property descriptors are non-writable and/or non-configurable or unassignable in some way (like stuffing them behind getters):

I also just verified that you are compiling to CJS, as you extend this config:

$ cat node_modules/\@tsconfig/node14/tsconfig.json
{
  "$schema": "https://json.schemastore.org/tsconfig",
  "display": "Node 14",

  "compilerOptions": {
    "lib": ["es2020"],
    "module": "commonjs",
    "target": "es2020",

    "strict": true,
    "esModuleInterop": true,
    "skipLibCheck": true,
    "forceConsistentCasingInFileNames": true,
    "moduleResolution": "node"
  }
}

So as you end up with CJS in any case, I modified the failing repro to just use require calls directly and presto it works and makes sense:

$ node index.js
{
  DEFAULT_OPTS: [Getter],
  ADB: [Getter],
  DEFAULT_ADB_PORT: [Getter],
  getAndroidBinaryPath: [Getter],
  getSdkRootFromEnv: [Getter],
  default: [class ADB]
}
{
  get: [Function: get],
  set: undefined,
  enumerable: true,
  configurable: true
}

(the additional output here is caused by me adding a console.dir(adb) and logging the object descriptor of the property to see how the module looks).

As you can see, after transpilation, getAndroidBinaryPath is no longer a normal field on the exported object, but a getter, essentially making it immutable if trying to assign anything to it 😢 Fortunately, Sinon comes with an API for working with getters and setters (docs) 🥳

And lo and behold, this works fine:

const adb = require("appium-adb");
const { createSandbox } = require("sinon");

const sandbox = createSandbox();
console.dir(adb);
console.log(Object.getOwnPropertyDescriptor(adb, "getAndroidBinaryPath"));
const yoloStub = sandbox.fake.resolves("yolo");
sandbox.stub(adb, "getAndroidBinaryPath").get(() => yoloStub);

adb
  .getAndroidBinaryPath("foobar")
  .then((val) => console.log("value: ", val))
  .catch((err) => console.log("failure", err));

This results in

...
value: yolo

[mykola-mokhnach]

@fatso83 Thank you for the very detailed investigation.

Would it make sense to improve the error message sinon throws in such case to make it more helpful?

[fatso83]

What do you suggest should happen given the context? Sinon does not know anything about the environment or what you intend to do. We could give some suggestions on what causes the current error, perhaps? Like

pseudocode

if trying to to wrap an undefined property; then
   if property name has a getter; then
        output hint that the property `propName` is undefined, but it is defined as a getter, so you might want to try `sandbox.stub(obj, propName).get(() => customStub)`
  else 
       output a generic error about undefined property
  endif
endif

[mykola-mokhnach]

yes, I like the idea above