Gracefully handle a missing subprocess module
#775
Conversation
As reported in #773, it seems that certain sandboxed environments like CloudFlare Workers don't provide a `subprocess` module, and that causes Node to throw when we try to require it. Because the only thing we need `subprocess` for is getting `uname`, here we extend the already-existing infrastructure built to gracefully handle errors to also be able to handle this new error case. Fixes #773.
brandur-stripe
force-pushed
the
brandur-handle-missing-subprocess
branch
from
64b4b9f
to
9e1f489
Compare
|
Had a brief conversation about this with @ob-stripe just now, and he mentioned that the alternative to this fix might be to suggest people stub out |
| utils.safeExec('hello', myCb); | ||
| expect(actualErr.toString()).to.equal( | ||
| new Error('exec not available').toString() | ||
| ); |
There was a problem hiding this comment.
I'd love if someone who is familiar with Node convention could check whether they like this. Basically I found that errors are not treated as equal by Chai even if they have the same error message, so fell back to this toString() solution, but there might be a better way.
There was a problem hiding this comment.
This works. We should upgrade our testing library to Jest.
| utils.safeExec('hello', myCb); | ||
| expect(actualErr.toString()).to.equal( | ||
| new Error('exec not available').toString() | ||
| ); |
There was a problem hiding this comment.
This works. We should upgrade our testing library to Jest.
Co-Authored-By: Alex Rattray (Stripe) <[email protected]>
|
Thank you Alex for going above and beyond the line of duty! ptal @ob-stripe Going to hand off to you for a final decision on whether this is the right way to go for the library. |
|
This PR makes it so ptal @brandur-stripe |
|
Good question OB. If you look carefully at how utils.safeExec('uname -a', (err, uname) => {
const userAgent = {};
for (const field in seed) {
userAgent[field] = encodeURIComponent(seed[field]);
}
// URI-encode in case there are unusual characters in the system's uname.
userAgent.uname = encodeURIComponent(uname || 'UNKNOWN');
if (this._appInfo) {
userAgent.application = this._appInfo;
}
cb(JSON.stringify(userAgent));
});
},Not sure if that's the best design, but it'll work at least. ptal @ob-stripe |
|
Thanks Olivier! |
As reported in #773, it seems that certain sandboxed environments like
CloudFlare Workers don't provide a
subprocessmodule, and that causesNode to throw when we try to require it.
Because the only thing we need
subprocessfor is gettinguname, herewe extend the already-existing infrastructure built to gracefully handle
errors to also be able to handle this new error case.
Fixes #773.
r? @rattrayalex-stripe (Or recommend another reviewer if you like.)
cc @stripe/api-libraries