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

Node 20: errors with util.inspect.custom raised in loader thread not serialized/deserialized correctly #48207

Closed
danrr opened this issue May 27, 2023 · 11 comments · Fixed by #48306

Comments

@danrr
Copy link

danrr commented May 27, 2023

Version

v20.2.0

Platform

No response

Subsystem

No response

What steps will reproduce the bug?

Throw an error with a custom inspect symbol in worker thread, which is then serialised and deserialised to be displayed by main thread.

Investigating a bug in ts-node (TypeStrong/ts-node#2026) I ran into some issues with how errors in (loader) worker threads are handled.

  1. util.inspect.custom is only checked in own properties (https://github.com/nodejs/node/blob/main/lib/internal/error_serdes.js#L137), ignoring it if the error was created using a class constructor (see https://github.com/TypeStrong/ts-node/blob/7af5c48864b60576e471da03c064f325ce37d850/src/index.ts#L432-L464)
  2. Errors with custom inspect methods are not displayed correctly by call to internalBinding('errors').triggerUncaughtException
node:internal/process/esm_loader:42
      internalBinding('errors').triggerUncaughtException(
                                ^
[Object: null prototype] {
  [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]]
}
  1. Errors defined by classes inheriting from Error don't appear to trigger the check here: https://github.com/nodejs/node/blob/main/lib/internal/error_serdes.js#L120
let a = new Error("hmm?")
Object.prototype.toString(a)
> "[object Object]"

How often does it reproduce? Is there a required condition?

Every time an error is raised in loader worker thread.

What is the expected behavior? Why is that the expected behavior?

Errors using custom inspect symbol raised in threads should serialise correctly

What do you see instead?

[Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]]

Additional information

No response

@aduh95
Copy link
Contributor

aduh95 commented May 28, 2023

The title says "loader thread", but I assume that's true for any worker thread, right?

@danrr
Copy link
Author

danrr commented May 28, 2023

One of the issues seems specific to the error handling code in process/esm_loader. Others seem to affect all error serialisation.

@aduh95
Copy link
Contributor

aduh95 commented May 30, 2023

Can you share a minimal repro?

@danrr
Copy link
Author

danrr commented May 30, 2023

@aduh95 would the repro I made for node-ts do?

TypeStrong/ts-node-repros#33

@aduh95
Copy link
Contributor

aduh95 commented May 30, 2023

A minimal repro would ideally not include any code to be downloaded from the internet, ideally a list of commands that can be copy pasted from the web browser to a terminal.

@aduh95
Copy link
Contributor

aduh95 commented Jun 2, 2023

  1. util.inspect.custom is only checked in own properties

I've opened #48306 to address this.

2. Errors with custom inspect methods are not displayed correctly by call to internalBinding('errors').triggerUncaughtException

It has nothing to do with the custom inspect method, triggerUncaughtException will not run any JS, it generates the error message using v8::Exception::createMessage so I think what you see is the expected output.

node/src/node_errors.cc

Lines 1005 to 1017 in 6700aac

static void TriggerUncaughtException(const FunctionCallbackInfo<Value>& args) {
Isolate* isolate = args.GetIsolate();
Environment* env = Environment::GetCurrent(isolate);
Local<Value> exception = args[0];
Local<Message> message = Exception::CreateMessage(isolate, exception);
if (env != nullptr && env->abort_on_uncaught_exception()) {
ReportFatalException(
env, exception, message, EnhanceFatalException::kEnhance);
Abort();
}
bool from_promise = args[1]->IsTrue();
errors::TriggerUncaughtException(isolate, exception, message, from_promise);
}

3. Errors defined by classes inheriting from Error don't appear to trigger the check here: https://github.com/nodejs/node/blob/main/lib/internal/error_serdes.js#L120

let a = new Error("hmm?")
Object.prototype.toString(a)
> "[object Object]"

You forgot .call, try e.g. Object.prototype.toString.call(new class A extends TypeError{}).


I'm not sure what this ticket is about, could you please share a minimal repro that includes steps to reproduce the issue (without needing to download code from the internet), the expected output, and what you see instead please?

nodejs-github-bot pushed a commit that referenced this issue Jun 11, 2023
PR-URL: #48306
Fixes: #48207
Reviewed-By: James M Snell <jasnell@gmail.com>
RafaelGSS pushed a commit that referenced this issue Jul 3, 2023
PR-URL: #48306
Fixes: #48207
Reviewed-By: James M Snell <jasnell@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48306
Fixes: nodejs#48207
Reviewed-By: James M Snell <jasnell@gmail.com>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
PR-URL: nodejs#48306
Fixes: nodejs#48207
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Aug 29, 2023
PR-URL: #48306
Fixes: #48207
Reviewed-By: James M Snell <jasnell@gmail.com>
ruyadorno pushed a commit that referenced this issue Sep 1, 2023
PR-URL: #48306
Fixes: #48207
Reviewed-By: James M Snell <jasnell@gmail.com>
@WalterWoshid
Copy link

Pointless issue. Still receive an error and only this output:

node:internal/process/esm_loader:40
      internalBinding('errors').triggerUncaughtException(
                                ^
[Object: null prototype] {
  [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]]
}

Node.js v18.19.0
error Command failed.
Exit code: 1

@pencilcheck
Copy link

pencilcheck commented Dec 31, 2023

Getting the same error on node 20. If I change it to mjs it runs perfectly, but for some reason it just can't run in esm loader. Tested in both node 18 and 20.

Changing this in tsconfig.json seems to fixed the issue for me (shows actual error)

  "ts-node": {
    "experimentalSpecifierResolution": "node",
    "transpileOnly": true,
    "esm": true,
  },

@coeing
Copy link

coeing commented Jan 8, 2024

Getting the same error on node 20. If I change it to mjs it runs perfectly, but for some reason it just can't run in esm loader. Tested in both node 18 and 20.

Changing this in tsconfig.json seems to fixed the issue for me (shows actual error)

  "ts-node": {
    "experimentalSpecifierResolution": "node",
    "transpileOnly": true,
    "esm": true,
  },

Using "transpileOnly": true is already enough for me to make it work.

@malgorzataxkowal
Copy link

malgorzataxkowal commented Jan 26, 2024

I had similar error:
node:internal/process/esm_loader:40 internalBinding('errors').triggerUncaughtException( ^ [Object: null prototype] { [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]] }
I added below suggestion about ts-node to my tsconfig.json and I saw details about the problem. I had changed module directory before and didn't noticed error. I changed directory in import statement and application started up.

@danijezernik
Copy link

I have the same error when i run Cypress in my project. It doesn't want to open and when it runs i get this error:

node:internal/modules/run_main:129
    triggerUncaughtException(
    ^
[Object: null prototype] {
  [Symbol(nodejs.util.inspect.custom)]: [Function: [nodejs.util.inspect.custom]]
}

No tsconfig options solved this issue. Any resloves yet on this? I tried to run in Node 18 and 20 but the same problem. here are the dependencies:

"dependencies": {
    "@aparajita/capacitor-biometric-auth": "^8.0.0",
    "@capacitor-community/file-opener": "^6.0.0",
    "@capacitor-firebase/analytics": "^6.0.0",
    "@capacitor/android": "^6.1.0",
    "@capacitor/app": "^6.0.0",
    "@capacitor/browser": "^6.0.1",
    "@capacitor/core": "^6.1.0",
    "@capacitor/filesystem": "^6.0.0",
    "@capacitor/ios": "^6.1.0",
    "@capacitor/share": "^6.0.1",
    "@capacitor/splash-screen": "^6.0.1",
    "@capacitor/storage": "^1.2.5",
    "@tanstack/vue-query": "^4.37.1",
    "@unocss/reset": "^0.53.6",
    "@vuelidate/core": "^2.0.3",
    "@vuelidate/validators": "^2.0.4",
    "@vueuse/core": "^10.11.0",
    "axios": "^1.7.2",
    "capacitor-blob-writer": "^1.1.16",
    "firebase": "^10.12.3",
    "floating-vue": "2.0.0-beta.24",
    "h3": "^1.12.0",
    "highcharts": "^10.3.3",
    "lodash-es": "^4.17.21",
    "naive-ui": "^2.38.2",
    "sass": "^1.77.6",
    "sass-loader": "^13.3.3",
    "swiper": "^9.4.1",
    "vite": "4.4.11",
    "vue": "^3.4.38",
    "vue-query": "^1.26.0",
    "vue-router": "^4.4.0",
    "vuelidate": "^0.7.7",
    "webpack": "^5.92.1"
  },
  "devDependencies": {
    "@antfu/eslint-config": "^1.2.1",
    "@capacitor/assets": "3.0.1",
    "@capacitor/cli": "^6.1.0",
    "@iconify-json/carbon": "^1.1.36",
    "@iconify-json/twemoji": "^1.1.15",
    "@nuxt/image": "1.1.0",
    "@nuxt/ui-templates": "^1.3.4",
    "@nuxtjs/color-mode": "^3.4.2",
    "@pinia/nuxt": "^0.4.11",
    "@types/vuelidate": "^0.7.21",
    "@unhead/vue": "^1.9.15",
    "@unocss/nuxt": "^0.46.5",
    "@vueuse/nuxt": "^9.13.0",
    "cypress": "^13.14.2",
    "eslint": "^8.57.0",
    "nuxt": "3.13.0",
    "pinia": "^2.1.7",
    "typescript": "^4.9.5"
  }

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

Successfully merging a pull request may close this issue.

7 participants