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

In worker threads, process.env is case-sensitive on windows #48955

Closed
leumasme opened this issue Jul 28, 2023 · 2 comments · Fixed by #49008
Closed

In worker threads, process.env is case-sensitive on windows #48955

leumasme opened this issue Jul 28, 2023 · 2 comments · Fixed by #49008
Labels
worker Issues and PRs related to Worker support.

Comments

@leumasme
Copy link

leumasme commented Jul 28, 2023

Version

v18.17.0

Platform

Microsoft Windows NT 10.0.19042.0 x64

Subsystem

worker_threads / process

What steps will reproduce the bug?

  • Set the environment variable "fookey" to "barvalue" (or any other variable name/value)
  • Create the following files in a fresh directory:
// main.js
const { Worker } = require("worker_threads");
const path = require("path");

const worker = new Worker(path.join(__dirname, "worker.js"));

console.log("in parent:")
console.log("barkey:",process.env.barkey)
console.log("BARKEY:",process.env.BARKEY)

worker.on("message", (data) => {
    console.log("in worker:")
    console.log("barkey:", data.barkey);
    console.log("BARKEY:", data.BARKEY);
});
// worker.js
const { parentPort } = require('worker_threads');

parentPort.postMessage({
  barkey: process.env.barkey,
  BARKEY: process.env.BARKEY,
});
  • Run main.js using node: node main.js

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

Always reproduces (on windows)

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

On windows, environmentvariables are case-insensitive.
Thus, we would expect to see the values of "barkey" and "BARKEY" to be equal within both the main context and the worker context.

in parent:
barkey: foovalue
BARKEY: foovalue
in worker:
barkey: foovalue
BARKEY: foovalue

What do you see instead?

In the main context, the values are equal, but in the worker context, only fookey (which has the same casing as the env var we set) returns a value

in parent:
barkey: foovalue
BARKEY: foovalue
in worker:
barkey: foovalue
BARKEY: undefined

Additional information

process.env is meant to be case-insensitive as documented in the process docs here:

On Windows operating systems, environment variables are case-insensitive.

import { env } from 'node:process';

env.TEST = 1;
console.log(env.test);
// => 1

The worker_theads docs don't mention an exception to this process.env behavior, so one would assume it applies there too.

Related issue that lead to the discovery of this behavior: httptoolkit/httptoolkit-server#91

@debadree25 debadree25 added the worker Issues and PRs related to Worker support. label Jul 30, 2023
nodejs-github-bot pushed a commit that referenced this issue Aug 5, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: #49008
Fixes: #48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@daeyeon
Copy link
Member

daeyeon commented Aug 8, 2023

Reopening for another opinion on #49008

@daeyeon daeyeon reopened this Aug 8, 2023
martenrichter pushed a commit to martenrichter/node that referenced this issue Aug 13, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#49008
Fixes: nodejs#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#49008
Fixes: nodejs#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Ceres6 pushed a commit to Ceres6/node that referenced this issue Aug 14, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#49008
Fixes: nodejs#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
UlisesGascon pushed a commit to UlisesGascon/node that referenced this issue Aug 14, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#49008
Fixes: nodejs#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
RafaelGSS pushed a commit that referenced this issue Aug 15, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: #49008
Fixes: #48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
RafaelGSS pushed a commit to RafaelGSS/node that referenced this issue Aug 15, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#49008
Fixes: nodejs#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
rluvaton pushed a commit to rluvaton/node that referenced this issue Aug 15, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs#49008
Fixes: nodejs#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
RafaelGSS pushed a commit that referenced this issue Aug 17, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: #49008
Fixes: #48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
@bnoordhuis
Copy link
Member

This can be closed, right? #49008 was merged and released if I'm not mistaken.

targos pushed a commit that referenced this issue Nov 27, 2023
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: #49008
Fixes: #48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs/node#49008
Fixes: nodejs/node#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
sercher added a commit to sercher/graaljs that referenced this issue Apr 25, 2024
Signed-off-by: Daeyeon Jeong <[email protected]>
PR-URL: nodejs/node#49008
Fixes: nodejs/node#48955
Reviewed-By: Luigi Pinca <[email protected]>
Reviewed-By: Ben Noordhuis <[email protected]>
Reviewed-By: Deokjin Kim <[email protected]>
Reviewed-By: Yagiz Nizipli <[email protected]>
Reviewed-By: Darshan Sen <[email protected]>
Reviewed-By: Tobias Nießen <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
worker Issues and PRs related to Worker support.
Projects
None yet
4 participants