Skip to content

Commit 6bf475c

Browse files
Jobianscharmander
andauthored
Improve Deno compatibility: config-first and safe env access (#3547)
* Set default PostgreSQL user to 'postgres' to avoid Deno env access errors Currently, the pg package defaults the database user to process.env.USER (or USERNAME on Windows). This causes errors when running in Deno without --allow-env, as environment access is restricted. This PR changes the default user to 'postgres', so: - Node.js behavior is unchanged when a user is explicitly provided in config. - Deno users no longer hit NotCapable errors for environment access. - Avoids relying on process.env for default values. Example: Before: user: process.platform === 'win32' ? process.env.USERNAME : process.env.USER, After: user: 'postgres', // default user, avoids using process.env * Prioritize config values over environment variables in ConnectionParameters Previously, ConnectionParameters would first check process.env for connection settings before using the provided config object. This could cause errors in environments like Deno where environment access is restricted. This change updates the val function to: 1. Use the value from config if it is defined. 2. Fall back to environment variables only if config does not provide a value. 3. Use default values if neither config nor environment variables are set. This ensures that explicitly provided configuration values are always respected, avoiding unnecessary errors in restricted environments. * Wrap NODE_PG_FORCE_NATIVE check in try/catch for Deno compatibility Replace the `typeof process.env.NODE_PG_FORCE_NATIVE !== 'undefined'` check with a try/catch to safely handle environments like Deno without `--allow-env`. - Keeps Node.js behavior unchanged. - Prevents errors when accessing process.env in Deno. - Preserves lazy loading of the native module. * Make default database user Deno-safe Wrap the default database user assignment in a try/catch to prevent errors when environment access is restricted (e.g., in Deno). - Uses process.env.USER / USERNAME on Node if available - Falls back to 'postgres' when env access fails or is unavailable - Maintains code structure and comments - Ensures Node tests continue to pass while preventing Deno runtime errors * Fixing checks pass * Update packages/pg/lib/connection-parameters.js Co-authored-by: Charmander <~@charmander.me> * fix(pg): only guard process.env access when forcing native client --------- Co-authored-by: Charmander <~@charmander.me>
1 parent d10e09c commit 6bf475c

3 files changed

Lines changed: 46 additions & 26 deletions

File tree

packages/pg/lib/connection-parameters.js

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,10 @@ const defaults = require('./defaults')
77
const parse = require('pg-connection-string').parse // parses a connection string
88

99
const val = function (key, config, envVar) {
10+
if (config[key]) {
11+
return config[key]
12+
}
13+
1014
if (envVar === undefined) {
1115
envVar = process.env['PG' + key.toUpperCase()]
1216
} else if (envVar === false) {
@@ -15,7 +19,7 @@ const val = function (key, config, envVar) {
1519
envVar = process.env[envVar]
1620
}
1721

18-
return config[key] || envVar || defaults[key]
22+
return envVar || defaults[key]
1923
}
2024

2125
const readSSLConfigFromEnvironment = function () {

packages/pg/lib/defaults.js

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,18 @@
11
'use strict'
22

3+
let user
4+
try {
5+
user = process.platform === 'win32' ? process.env.USERNAME : process.env.USER
6+
} catch {
7+
// ignore, e.g., Deno without --allow-env
8+
}
9+
310
module.exports = {
411
// database host. defaults to localhost
512
host: 'localhost',
613

714
// database user's name
8-
user: process.platform === 'win32' ? process.env.USERNAME : process.env.USER,
15+
user,
916

1017
// name of database to connect
1118
database: undefined,

packages/pg/lib/index.js

Lines changed: 33 additions & 24 deletions
Original file line numberDiff line numberDiff line change
@@ -34,31 +34,40 @@ const PG = function (clientConstructor) {
3434
this.utils = utils
3535
}
3636

37-
if (typeof process.env.NODE_PG_FORCE_NATIVE !== 'undefined') {
38-
module.exports = new PG(require('./native'))
39-
} else {
40-
module.exports = new PG(Client)
37+
let clientConstructor = Client
4138

42-
// lazy require native module...the native module may not have installed
43-
Object.defineProperty(module.exports, 'native', {
44-
configurable: true,
45-
enumerable: false,
46-
get() {
47-
let native = null
48-
try {
49-
native = new PG(require('./native'))
50-
} catch (err) {
51-
if (err.code !== 'MODULE_NOT_FOUND') {
52-
throw err
53-
}
39+
let forceNative = false
40+
try {
41+
forceNative = !!process.env.NODE_PG_FORCE_NATIVE
42+
} catch {
43+
// ignore, e.g., Deno without --allow-env
44+
}
45+
46+
if (forceNative) {
47+
clientConstructor = require('./native')
48+
}
49+
50+
module.exports = new PG(clientConstructor)
51+
52+
// lazy require native module...the native module may not have installed
53+
Object.defineProperty(module.exports, 'native', {
54+
configurable: true,
55+
enumerable: false,
56+
get() {
57+
let native = null
58+
try {
59+
native = new PG(require('./native'))
60+
} catch (err) {
61+
if (err.code !== 'MODULE_NOT_FOUND') {
62+
throw err
5463
}
64+
}
5565

56-
// overwrite module.exports.native so that getter is never called again
57-
Object.defineProperty(module.exports, 'native', {
58-
value: native,
59-
})
66+
// overwrite module.exports.native so that getter is never called again
67+
Object.defineProperty(module.exports, 'native', {
68+
value: native,
69+
})
6070

61-
return native
62-
},
63-
})
64-
}
71+
return native
72+
},
73+
})

0 commit comments

Comments
 (0)