Skip to content

Commit

Permalink
Fix non-z.request logging (#387)
Browse files Browse the repository at this point in the history
  • Loading branch information
xavdid authored Jun 11, 2021
1 parent e1f5098 commit 504607f
Show file tree
Hide file tree
Showing 3 changed files with 25 additions and 17 deletions.
5 changes: 3 additions & 2 deletions packages/core/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -18,10 +18,11 @@
"preversion": "git pull && yarn test",
"version": "node bin/bump-dependencies.js && yarn && git add package.json yarn.lock",
"postversion": "git push && git push --tags",
"test": "mocha -t 20000 --recursive test",
"main-tests": "mocha -t 20000 --recursive test",
"solo-test": "test $(OPT_OUT_PATCH_TEST_ONLY=yes mocha --recursive test -g 'should be able to opt out of patch' -R json | jq '.stats.passes') -eq 1 && echo 'Ran 1 test and it passed!'",
"test": "yarn main-tests && yarn solo-test",
"debug": "mocha -t 10000 --inspect-brk --recursive test",
"test:w": "mocha -t 10000 --recursive test --watch",
"plain-test": "mocha -t 5000 --recursive test",
"integration-test": "mocha -t 20000 integration-test",
"local-integration-test": "mocha -t 10000 integration-test --local",
"lambda-integration-test": "mocha -t 10000 integration-test --lambda",
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/tools/create-lambda-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -205,7 +205,8 @@ const createLambdaHandler = (appRawOrPath) => {
// Adds logging for _all_ kinds of http(s) requests, no matter the library
if (!skipHttpPatch) {
const httpPatch = createHttpPatch(event);
httpPatch(require('http')); // 'https' uses 'http' under the hood
httpPatch(require('http'));
httpPatch(require('https')); // 'https' needs to be patched separately
}

// TODO: Avoid calling prepareApp(appRaw) repeatedly here as createApp()
Expand Down
34 changes: 20 additions & 14 deletions packages/core/test/tools/http-patch.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,25 +5,31 @@ const should = require('should');
const createAppTester = require('../../src/tools/create-app-tester');
const appDefinition = require('../userapp');

describe.skip('create-lambda-handler', () => {
// this block is skipped because there's no way to un-modify 'http' once we've done it
// I've verified that the bottom test works in isolation, but doesn't when it's part of the larger suite
describe('create-lambda-handler', () => {
describe('http patch', () => {
it('should patch by default', async () => {
const appTester = createAppTester(appDefinition);
await appTester(appDefinition.resources.list.list.operation.perform);
const http = require('http'); // core modules are never cached
should(http.patchedByZapier).eql(true);
should(require('http').patchedByZapier).eql(true);
should(require('https').patchedByZapier).eql(true);
});

it('should be ablet opt out of patch', async () => {
const appTester = createAppTester({
...appDefinition,
flags: { skipHttpPatch: true },
});
await appTester(appDefinition.resources.list.list.operation.perform);
const http = require('http'); // core modules are never cached
should(http.patchedByZapier).eql(undefined);
});
// when we run this test, we have to run it without any other test calling createAppTester
// this block is skipped because there's no way to un-modify 'http' once we've done it
// it's skipped in the main suite and run on its own afterwards
// if the test name changes, the test command in package.json must as well
// console.error(process.env.OPT_OUT_PATCH_TEST_ONLY);
(process.env.OPT_OUT_PATCH_TEST_ONLY ? it : it.skip)(
'should be able to opt out of patch',
async () => {
const appTester = createAppTester({
...appDefinition,
flags: { skipHttpPatch: true },
});
await appTester(appDefinition.resources.list.list.operation.perform);
const http = require('http'); // core modules are never cached
should(http.patchedByZapier).eql(undefined);
}
);
});
});

0 comments on commit 504607f

Please sign in to comment.