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

error: internal error in file ../src/njsUtils.c, line 805 () #11851

Open
Luk4h opened this issue Jun 13, 2024 · 4 comments
Open

error: internal error in file ../src/njsUtils.c, line 805 () #11851

Luk4h opened this issue Jun 13, 2024 · 4 comments
Labels
bug Something isn't working needs triage

Comments

@Luk4h
Copy link

Luk4h commented Jun 13, 2024

What version of Bun is running?

1.1.13+bd6a60512

What platform is your computer?

Linux 6.5.0-35-generic x86_64 x86_64

What steps can reproduce the bug?

When trying to stablish a connection using OracleDb latest package in thick mode.

There is an example.js file in the package that is enough to reproduce the bug when passing the correct variables to use thick mode.

Running in thick mode
638 |     _initializeThinDriver();
639 |   }
640 |
641 |   const conn = new Connection();
642 |   conn._impl = new impl.ConnectionImpl();
643 |   await conn._impl.connect(options);
              ^
error: internal error in file ../src/njsUtils.c, line 805 ()
      at /home/***/Dev/***/node_modules/oracledb/lib/oracledb.js:643:9

What is the expected behavior?

To connect normally to the database.

What do you see instead?

Running in thick mode
638 |     _initializeThinDriver();
639 |   }
640 |
641 |   const conn = new Connection();
642 |   conn._impl = new impl.ConnectionImpl();
643 |   await conn._impl.connect(options);
              ^
error: internal error in file ../src/njsUtils.c, line 805 ()
      at /home/***/Dev/***/node_modules/oracledb/lib/oracledb.js:643:9

Additional information

I have also created an issue on OracleDb repository #1679, but since it shows the error from a C file the teams suggested for me to try get some support on Bun's repo as well.

@Luk4h Luk4h added bug Something isn't working needs triage labels Jun 13, 2024
@pg-wtatum
Copy link

I'm also facing the same issue -- looking to build a small utility application where bun compile would be an ideal why to distribute but I cannot proceed down that path due to blocker with oracledb in thick mode . A trivial reproduction is

import { createPool, initOracleClient } from "oracledb";

console.log("hello");
initOracleClient();
const pool = createPool({});
console.log("oracle");

Which produces this log output for me

> bun .\index.ts
hello
oracle
571 |
572 |   // create the pool, ensuring that the temporary pool alias cache is removed
573 |   // once this has completed (either successfully or unsuccessfully)
574 |   const pool = new Pool();
575 |   try {
576 |     await pool._impl.create(options);
                ^
error: internal error in file D:\git_files\node-oracledb\src\njsUtils.c, line 805 ()
 code: "internal error in file D"

      at C:\Users\wtatum\dev\code\scratch\node_modules\oracledb\lib\oracledb.js:576:11

Bun v1.1.10 (Windows x64)

Not sure if this is helpful but Deno can run the same file without any modification, and can successfully connect if the right credentials are given to the pool.

This is pretty easy to reproduce since you don't even need valid Oracle credentials (or a running server) to reach the crash.

Sorry the report isn't more helpful, I don't know enough about what's happening on the .c side of node, bun, or the oracle thick client to add much more there.

@pg-wtatum
Copy link

Any advice on how I could dig deeper into the root cause of this? Since the oracledb module runs fine in NJS (its intended runtime) and also in Deno I think it must be something specific to Bun but I feel like a callstack from njsUtils.c sort of represents a dead-end in what I can meaningfully debug.

@pg-wtatum
Copy link

pg-wtatum commented Aug 21, 2024

The only real insight I've made is that the error comes from

https://github.com/oracle/node-oracledb/blob/main/src/njsUtils.c#L805

which is in this function

//-----------------------------------------------------------------------------
// njsUtils_validateArgs()
//   Gets the instance associated with the object and gets the arguments as
// well. If the number of arguments is incorrect, an exception is thrown.
//-----------------------------------------------------------------------------
bool njsUtils_validateArgs(napi_env env, napi_callback_info info,
        size_t numArgs, napi_value *args, njsModuleGlobals **globals,
        napi_value *callingObj, const njsClassDef *classDef, void **instance)

But I can't really tell the cause of the throw just by reading it. It looks like a bad response from napi_wrap would be the cause

  if (napi_wrap(env, localCallingObj, *instance,
          classDef->finalizeFn, NULL, NULL) != napi_ok) {
      free(*instance);
      return njsUtils_genericThrowError(env, __FILE__, __LINE__);
  }

@Luk4h
Copy link
Author

Luk4h commented Aug 28, 2024

I delved deeper into the OracleDB module and its code. Here is the current call stack:

Their module is a JavaScript program that reads instructions from a pre-built .node program, which is built using C. They have an object Impl that interacts with this program running the C code. The instruction currently causing the error is conn._impl.connect(options), which calls the connect method on the connection class in the C code. They have set up a mechanism where, for each asynchronous call from Node.js, they create a "baton":

njsModule.h

// Define macro for asynchronous Node-API methods
...
#define NJS_NAPI_METHOD_IMPL_ASYNC(name, numArgs, classDef) \
    static napi_value name(napi_env env, napi_callback_info info) { \
        napi_value args[numArgs + 1], returnValue = NULL; \
        njsBaton *baton = NULL; \
        if (!njsUtils_createBaton(env, info, numArgs, args, classDef, &baton)) \
            return NULL; \
        if (!name##_(env, args, baton, &returnValue)) { \
            if (baton->deferred) { \
                napi_reject_deferred(env, baton->deferred, NULL); \
                njsBaton_free(baton, env); \
            } else { \
                njsBaton_reportError(baton, env); \
            } \
            return NULL; \
        } \
        return returnValue; \
    } \
    static bool name##_(napi_env env, napi_value* args, njsBaton *baton, \
            napi_value *returnValue)

Currently, the method njsUtils_createBaton(env, info, numArgs, args, classDef, &baton) is throwing the error:

njsBaton.c

//-----------------------------------------------------------------------------
// njsBaton_create()
//   Populates the baton with common information and performs common checks.
// This method should only be called by njsUtils_createBaton().
//-----------------------------------------------------------------------------
bool njsBaton_create(njsBaton *baton, napi_env env, napi_callback_info info,
        size_t numArgs, napi_value *args, const njsClassDef *classDef)
{
    napi_value callingObj;

    // Validate the number of args required for the asynchronous function
    // and get the calling instance
    if (!njsUtils_validateArgs(env, info, numArgs, args, &baton->globals,
            &callingObj, classDef, &baton->callingInstance))
        return false;

    // Save a reference to the calling object so that it will not be garbage
    // collected during the asynchronous call
    NJS_CHECK_NAPI(env, napi_create_reference(env, callingObj, 1,
            &baton->jsCallingObjRef))

    return true;
}

Specifically, !njsUtils_validateArgs(env, info, numArgs, args, &baton->globals, &callingObj, classDef, &baton->callingInstance) is the problematic method:

njsUtils.c

//-----------------------------------------------------------------------------
// njsUtils_validateArgs()
//   Gets the instance associated with the object and gets the arguments as
// well. If the number of arguments is incorrect, an exception is thrown.
//-----------------------------------------------------------------------------
bool njsUtils_validateArgs(napi_env env, napi_callback_info info,
        size_t numArgs, napi_value *args, njsModuleGlobals **globals,
        napi_value *callingObj, const njsClassDef *classDef, void **instance)
{
    napi_value localCallingObj;
    size_t actualArgs;

    // Get callback information and validate the number of arguments
    actualArgs = numArgs;
    NJS_CHECK_NAPI(env, napi_get_cb_info(env, info, &actualArgs, args,
            &localCallingObj, (void**) globals))
    if (actualArgs != numArgs)
        return njsUtils_genericThrowError(env, __FILE__, __LINE__);

    // Unwrap instance, if applicable
    if (callingObj)
        *callingObj = localCallingObj;
    if (instance) {
        if (classDef == &njsClassDefDbObject) {
            if (!njsDbObject_getInstance(*globals, env, localCallingObj,
                    (njsDbObject**) instance))
                return false;
        } else if (classDef) {
            *instance = calloc(1, classDef->structSize);
            if (!*instance)
                return njsUtils_throwInsufficientMemory(env);
            if (napi_wrap(env, localCallingObj, *instance,
                    classDef->finalizeFn, NULL, NULL) != napi_ok) {
                free(*instance);
                return njsUtils_genericThrowError(env, __FILE__, __LINE__); // <-- Last Throw.
            }
        } else {
            NJS_CHECK_NAPI(env, napi_unwrap(env, localCallingObj,
                    (void**) instance))
        }
    }
    return true;
}

This is all the information I have. Since it works with Deno, I speculate they created their implementation to work with this node_api.h, which is Node.js's own bindings for C, included in the njsModule.h file.

Which I guess ends with the necessity of Bun's own implementation of this Node bindings to work. They already have. I guess there is a problem in one of these bindings that is causing this error.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working needs triage
Projects
None yet
Development

No branches or pull requests

2 participants