-
Notifications
You must be signed in to change notification settings - Fork 35
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
fix: Better optional deps handling #353
Conversation
The odbc and idb are optional deps that might not be installed. When either is not available, return an error back to user instead of failing.
2b3b4a0
to
53f9bf1
Compare
lib/transports/idbTransport.js
Outdated
@@ -28,6 +28,11 @@ try { | |||
} | |||
|
|||
function idbCall(config, xmlInput, cb) { | |||
// idb transport is not available bail out | |||
if (idb === null) { | |||
cb(new Error('idb-connector module was not found, the idb transport is only usable on IBM i.'), null); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe something like
"idb-connector was not found, ensure idb-connector is installed properly (idb-connector is only available on IBM i)"
I get nervous about the term "module", since idb-connector doesn't have type: module
in its package.json, which affects all sorts of things (CJS vs. ESM modules)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good to me!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed up in: 12083c9
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
odbc and idb-connector are optional deps and might not be installed.
When either is not available, return an error back to user instead of failing.
Fixes #317