Skip to content

Commit

Permalink
fix: Do not load pino-pretty in production bundles
Browse files Browse the repository at this point in the history
Browser tests were broken since #10459 due to the static import of
pino-pretty, which had some browser unfriendly stuff. This PR removes
that import in favor of setting it up in a separate jest setup file,
only run under jest. This follows the recommendation of pino-pretty of
not importing it in production.

Note that, for end-users who want pretty logging, pino-pretty will still
kick in but as a transport, so it gets loaded in a nodejs worker thread
by pino itself, and does not pollute the static imports.

Another way of fixing this would have been adding the 'pino-pretty'
exception in the webpack fallbacks, but it felt wrong to start adding so
many deps in there.

Another option would have been to dynamically import (as opposed to
statically) pino-pretty, and use a webpackIgnore magic comment to skip
it. But it's unclear whether this would have worked with other bundlers,
and besides it would have added an async operation to our otherwise
synchronous logger setup.
  • Loading branch information
spalladino committed Dec 10, 2024
1 parent 323e2eb commit d0828c0
Show file tree
Hide file tree
Showing 43 changed files with 188 additions and 47 deletions.
5 changes: 4 additions & 1 deletion yarn-project/accounts/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/aztec.js": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/archiver/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuit-types": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/aztec-faucet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/ethereum": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/aztec-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/archiver": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/aztec.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuit-types": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/aztec/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"engines": {
"node": ">=18"
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/bb-prover/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuit-types": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/bot/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/accounts": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/builder/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/foundation": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/circuit-types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuits.js": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/circuits.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -98,6 +98,9 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
}
}
5 changes: 4 additions & 1 deletion yarn-project/cli-wallet/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/accounts": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/cli/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/archiver": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/end-to-end/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -156,6 +156,9 @@
},
"testRegex": "./src/.*\\.test\\.(js|mjs|ts)$",
"rootDir": "./src",
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
}
}
2 changes: 1 addition & 1 deletion yarn-project/end-to-end/src/shared/browser.ts
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ export const browserTestSuite = (
pageLogger.info(msg.text());
});
page.on('pageerror', err => {
pageLogger.error(err.toString());
pageLogger.error(`Error on web page`, err);
});
await page.goto(`${webServerURL}/index.html`);
while (!(await page.evaluate(() => !!window.AztecJs))) {
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/entrypoints/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/aztec.js": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/epoch-cache/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -87,7 +87,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"engines": {
"node": ">=18"
Expand Down
7 changes: 5 additions & 2 deletions yarn-project/ethereum/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,12 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"engines": {
"node": ">=18"
}
}
}
8 changes: 7 additions & 1 deletion yarn-project/foundation/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,13 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFilesAfterEnv": [
"../../foundation/src/jest/setup.mjs"
],
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/bb.js": "portal:../../barretenberg/ts",
Expand Down
8 changes: 8 additions & 0 deletions yarn-project/foundation/src/jest/setup.mjs
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
import { overwriteLoggingStream, pinoPrettyOpts } from '@aztec/foundation/log';
import pretty from 'pino-pretty';

// Overwrite logging stream with pino-pretty. We define this as a separate
// file so we don't mess up with dependencies in non-testing environments,
// since pino-pretty messes up with browser bundles.
// See also https://www.npmjs.com/package/pino-pretty?activeTab=readme#user-content-usage-with-jest
overwriteLoggingStream(pretty(pinoPrettyOpts));
20 changes: 15 additions & 5 deletions yarn-project/foundation/src/log/pino-logger.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
import { createColors } from 'colorette';
import isNode from 'detect-node';
import { pino, symbols } from 'pino';
import pretty from 'pino-pretty';
import { type Writable } from 'stream';
import { inspect } from 'util';

Expand Down Expand Up @@ -68,7 +67,7 @@ const [logLevel, logFilters] = parseEnv(process.env.LOG_LEVEL, defaultLogLevel);
// Transport options for pretty logging to stderr via pino-pretty.
const useColor = true;
const { bold, reset } = createColors({ useColor });
const pinoPrettyOpts = {
export const pinoPrettyOpts = {
destination: 2,
sync: true,
colorize: useColor,
Expand All @@ -78,6 +77,7 @@ const pinoPrettyOpts = {
customColors: 'fatal:bgRed,error:red,warn:yellow,info:green,verbose:magenta,debug:blue,trace:gray',
minimumLevel: 'trace' as const,
};

const prettyTransport: pino.TransportSingleOptions = {
target: 'pino-pretty',
options: pinoPrettyOpts,
Expand Down Expand Up @@ -113,11 +113,13 @@ const otelTransport: pino.TransportSingleOptions = {

function makeLogger() {
if (!isNode) {
// We are on the browser
// We are on the browser.
return pino({ ...pinoOpts, browser: { asObject: false } });
} else if (process.env.JEST_WORKER_ID) {
// We are on jest, so we need sync logging. We stream to stderr with pretty.
return pino(pinoOpts, pretty(pinoPrettyOpts));
// We are on jest, so we need sync logging and stream to stderr.
// We expect jest/setup.mjs to kick in later and replace set up a pretty logger,
// but if for some reason it doesn't, at least we're covered with a default logger.
return pino(pinoOpts, pino.destination(2));
} else {
// Regular nodejs with transports on worker thread, using pino-pretty for console logging if LOG_JSON
// is not set, and an optional OTLP transport if the OTLP endpoint is provided.
Expand All @@ -142,6 +144,14 @@ logger.verbose(
: `Browser console logger initialized with level ${logLevel}`,
);

/**
* Overwrites the logging stream with a different destination.
* Used by jest/setup.mjs to set up a pretty logger.
*/
export function overwriteLoggingStream(stream: Writable): void {
(logger as any)[symbols.streamSym] = stream;
}

/**
* Registers an additional destination to the pino logger.
* Use only when working with destinations, not worker transports.
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/ivc-integration/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -59,7 +59,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/bb.js": "../../ts",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/key-store/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuit-types": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/kv-store/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,9 @@
],
"testRegex": "./src/.*\\.test\\.(js|mjs|ts)$",
"rootDir": "./src",
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
}
}
5 changes: 4 additions & 1 deletion yarn-project/merkle-tree/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuit-types": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/noir-contracts.js/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/aztec.js": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/noir-protocol-circuits-types/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuits.js": "workspace:^",
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/p2p-bootstrap/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"engines": {
"node": ">=18"
Expand Down
5 changes: 4 additions & 1 deletion yarn-project/p2p/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -60,7 +60,10 @@
}
]
],
"testTimeout": 30000
"testTimeout": 30000,
"setupFiles": [
"../../foundation/src/jest/setup.mjs"
]
},
"dependencies": {
"@aztec/circuit-types": "workspace:^",
Expand Down
Loading

0 comments on commit d0828c0

Please sign in to comment.