Skip to content

Use console.warn/console.info where appropriate#20252

Merged
timvandermeij merged 1 commit intomozilla:masterfrom
sigmaSd:patch-1
Sep 16, 2025
Merged

Use console.warn/console.info where appropriate#20252
timvandermeij merged 1 commit intomozilla:masterfrom
sigmaSd:patch-1

Conversation

@sigmaSd
Copy link
Contributor

@sigmaSd sigmaSd commented Sep 9, 2025 •
edited
Loading

Why: the practical reason is I have a pdf reader mcp , that uses deno + pdfjs, it works but shows this warning Warning: Setting up fake worker.

the problem is that warning is printed to stdout so it breaks the mcp.

By switching to console.warn deno will print to stderr which make it work correctly.

Here is the code btw to make pdfjs work with deno

//pdfjs magic
import DOMMatrix from "npm:@thednp/dommatrix@2.0.12";
globalThis.DOMMatrix = DOMMatrix;
globalThis.process = undefined;
Object.defineProperty(navigator, "platform", {
  value: "Linux",
});
const pdfjsLib = await import("npm:pdfjs-dist@5.4.149");
pdfjsLib.GlobalWorkerOptions.workerSrc = import.meta.resolve("npm:pdfjs-dist@5.4.149/build/pdf.worker.mjs");
// pdfjs writes warning to stdout, which breaks mcp, this worksaround it
console.log = console.warn

DOMMatrix is expected to land on the next deno version (pr already done)

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Sep 9, 2025

The node legacy branch obviously works but it requires ffi (for npm canvas) while this one require no permission

timvandermeij
timvandermeij approved these changes Sep 14, 2025
View reviewed changes
Copy link
Contributor

@timvandermeij timvandermeij left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good to me, with the comment addressed and passing tests. Note that quite a few years ago in #10234 this approach was rejected, but revisiting this now it seems like a good idea to use the stderr/stdout streams for their intended purposes (in addition to the verbosity setting).

Please update the commit message to shortly describe why this change is beneficial for future context too.

@sigmaSd sigmaSd force-pushed the patch-1 branch 2 times, most recently from 324a64c to ef72c19 Compare September 15, 2025 22:34
@sigmaSd
Copy link
Contributor Author

sigmaSd commented Sep 15, 2025

Thanks I addressed your comments

@sigmaSd
use console.warn/info where appropriate
b6165c0 
Change info to use console.info and warn function
to use console.warn, this not only makes sense semantically
but also in practice server side runtimes like deno
write console.log to stdout, and console.warn
to stderr (info goes to stdout, unfortunately?)
this is important because logging to stdout
can break some cli apps.
@timvandermeij timvandermeij changed the title use console.warn instead of console.log Use console.warn/console.info where appropriate Sep 16, 2025
@timvandermeij
Copy link
Contributor

timvandermeij commented Sep 16, 2025

/botio unittest

@moz-tools-bot
Copy link
Collaborator

moz-tools-bot commented Sep 16, 2025

From: Bot.io (Linux m4)

Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.241.84.105:8877/75a0cd0d6e21b96/output.txt

@moz-tools-bot
Copy link
Collaborator

moz-tools-bot commented Sep 16, 2025

From: Bot.io (Windows)

Received

Command cmd_unittest from @timvandermeij received. Current queue size: 0

Live output at: http://54.193.163.58:8877/c269f45d2a0af80/output.txt

@moz-tools-bot
Copy link
Collaborator

moz-tools-bot commented Sep 16, 2025

From: Bot.io (Linux m4)

Success

Full output at http://54.241.84.105:8877/75a0cd0d6e21b96/output.txt

Total script time: 2.92 mins

  • Unit Tests: Passed

@moz-tools-bot
Copy link
Collaborator

moz-tools-bot commented Sep 16, 2025

From: Bot.io (Windows)

Failed

Full output at http://54.193.163.58:8877/c269f45d2a0af80/output.txt

Total script time: 8.44 mins

  • Unit Tests: FAILED

@timvandermeij timvandermeij merged commit 05af4ff into mozilla:master Sep 16, 2025
9 checks passed
@timvandermeij
Copy link
Contributor

timvandermeij commented Sep 16, 2025

Thanks!

@sigmaSd
Copy link
Contributor Author

sigmaSd commented Sep 16, 2025

Thanks as well!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@timvandermeij timvandermeij timvandermeij approved these changes

Assignees

No one assigned

Labels

core

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@sigmaSd @timvandermeij @moz-tools-bot