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 crash when requiring a missing package #56538

Open
benjaminjkraft opened this issue Jan 9, 2025 · 0 comments
Open

Node crash when requiring a missing package #56538

benjaminjkraft opened this issue Jan 9, 2025 · 0 comments

Comments

@benjaminjkraft
Copy link

Version

22.13.0

Platform

Linux a747a355dcc5 6.2.0-1018-aws #18~22.04.1-Ubuntu SMP Wed Jan 10 22:54:16 UTC 2024 x86_64 GNU/Linux

Subsystem

No response

What steps will reproduce the bug?

Build the following Dockerfile (e.g. docker build .) on linux (x86?):

FROM node:22.13.0-bookworm-slim
WORKDIR /srv/test

# === not needed for repro, just for debugging convenience
RUN apt update
RUN apt install -y vim-tiny
RUN chmod a+w .
# ===

# make sure that some directory in the module search path is inaccessible to the current user
USER node
ENV HOME=/root

# install needed packages; do *not* install @opentelemetry/winston-transport -- that require is the one that crashes things when it fails
RUN npm install @opentelemetry/instrumentation-tedious @opentelemetry/instrumentation-winston snowflake-sdk

# run the repro; both instrumentations are needed seemingly
RUN cat <<EOF >test.js
const { TediousInstrumentation } = require("@opentelemetry/instrumentation-tedious")
const { WinstonInstrumentation } = require("@opentelemetry/instrumentation-winston") // patches `winston`
new WinstonInstrumentation({})
new TediousInstrumentation({})
require("snowflake-sdk")  // ultimately imports calls the code patched by `instrumentation-winston`
EOF
RUN node --min-semi-space-size=128 test.js

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

There seem to be a bunch of required conditions:

  • we are requiring a package that does not exist
  • a directory in the node_modules search path is inaccessible to the current user
  • this is on an AWS m5 linux instance; I couldn't reproduce on a Mac M3, even via Docker

But there are some more conditions that I don't understand too. For example in the above repro I have no idea why the --min-semi-space-size flag, or the instrumentation-tedious import, are necessary, nor why replacing require("snowflake-sdk") with, say, require("winston").createLogger({}) doesn't repro (we must need to import some other things first, I guess). This is as minimal as I've managed to get the example.

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

The process should complete successfully.

In particular, this require should indeed fail (the package is not installed), but should throw an ordinary JS exception, which is caught a few lines below, allowing the rest of the program to proceed:
https://github.com/open-telemetry/opentelemetry-js-contrib/blob/3350583fd4b0b2e08ed20429bfc5409d537d7a9d/plugins/node/opentelemetry-instrumentation-winston/src/instrumentation.ts#L194

What do you see instead?

The process crashes when executing the above-linked line, with the following message:

terminate called after throwing an instance of 'std::filesystem::__cxx11::filesystem_error'
  what():  filesystem error: status: Permission denied [/root/.node_modules]
Aborted (core dumped)

Additional information

This seems to have been introduced between 22.8.0 and 22.9.0. It doesn't repro on any of the 23.x versions I tried.

In our actual case we can likely work around this by avoiding having any inaccessible directory on the node_modules path, but I wanted to report it as well!

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

No branches or pull requests

1 participant