Skip to content

Commit

Permalink
fix: move the Windows C++ redistributable warning so it is only shown…
Browse files Browse the repository at this point in the history
… if there is an actual access violation (cloudflare#6508)

Replaces cloudflare#6471, which was too verbose.

Fixes cloudflare#6170
  • Loading branch information
petebacondarwin authored Aug 16, 2024
1 parent 14fb034 commit 56a3de2
Show file tree
Hide file tree
Showing 7 changed files with 48 additions and 15 deletions.
9 changes: 9 additions & 0 deletions .changeset/tasty-mayflies-march.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
---
"wrangler": patch
---

fix: move the Windows C++ redistributable warning so it is only shown if there is an actual access violation

Replaces #6471, which was too verbose.

Fixes #6170
1 change: 1 addition & 0 deletions .vscode/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@
"PKCE",
"Positionals",
"qwik",
"Redistributable",
"reinitialise",
"scandir",
"selfsigned",
Expand Down
4 changes: 4 additions & 0 deletions fixtures/worker-app/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,10 @@ import { logErrors } from "./log";

console.log("startup log");

console.error(
"*** Received structured exception #0xc0000005: access violation; stack: 7ffe71872f57 7ff7834b643b 7ff7834b643b"
);

/** @param {Uint8Array} array */
function hexEncode(array) {
return Array.from(array)
Expand Down
7 changes: 7 additions & 0 deletions fixtures/worker-app/tests/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,13 @@ describe("'wrangler dev' correctly renders pages", () => {
expect(output).toContain("startup log");
expect(output).toContain("request log");

if (process.platform === "win32") {
// Check that the Windows warning is shown for the fake access violation error
expect(output).toContain(
"On Windows, this may be caused by an outdated Microsoft Visual C++ Redistributable library."
);
}

// check host on request in the Worker is as expected
expect(output).toContain(`host' => 'prod.example.org`);

Expand Down
9 changes: 0 additions & 9 deletions packages/wrangler/src/dev/local.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -228,15 +228,6 @@ function useLocalWorker(props: LocalProps) {
"code" in error &&
(error as { code: string }).code === "ERR_RUNTIME_FAILURE"
) {
// In the past we have seen Access Violation errors on Windows, which may be caused by an outdated
// version of the Microsoft Visual C++ Redistributable.
// See https://github.com/cloudflare/workers-sdk/issues/6170#issuecomment-2245209918
if (process.platform === "win32") {
logger.error(
"Check that you have the latest Microsoft Visual C++ Redistributable library installed.\n" +
"See https://learn.microsoft.com/en-us/cpp/windows/latest-supported-vc-redist."
);
}
// Don't log a full verbose stack-trace when Miniflare 3's workerd instance fails to start.
// workerd will log its own errors, and our stack trace won't have any useful information.
logger.error(String(error));
Expand Down
31 changes: 26 additions & 5 deletions packages/wrangler/src/dev/miniflare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -728,6 +728,9 @@ export function handleRuntimeStdio(stdout: Readable, stderr: Readable) {
isCodeMovedWarning(chunk: string) {
return /CODE_MOVED for unknown code block/.test(chunk);
},
isAccessViolation(chunk: string) {
return chunk.includes("access violation;");
},
};

stdout.on("data", (chunk: Buffer | string) => {
Expand All @@ -752,7 +755,7 @@ export function handleRuntimeStdio(stdout: Readable, stderr: Readable) {
logger.warn(chunk);
}

// anything not exlicitly handled above should be logged as info (via stdout)
// anything not explicitly handled above should be logged as info (via stdout)
else {
logger.info(getSourceMappedString(chunk));
}
Expand All @@ -775,15 +778,33 @@ export function handleRuntimeStdio(stdout: Readable, stderr: Readable) {
`Address already in use (${address}). Please check that you are not already running a server on this address or specify a different port with --port.`
);

// even though we've intercepted the chunk and logged a better error to stderr
// fallthrough to log the original chunk to the debug log file for observability
// Log the original error to the debug logs.
logger.debug(chunk);
}
// In the past we have seen Access Violation errors on Windows, which may be caused by an outdated
// version of the Windows OS or the Microsoft Visual C++ Redistributable.
// See https://github.com/cloudflare/workers-sdk/issues/6170#issuecomment-2245209918
else if (classifiers.isAccessViolation(chunk)) {
let error = "There was an access violation in the runtime.";
if (process.platform === "win32") {
error +=
"\nOn Windows, this may be caused by an outdated Microsoft Visual C++ Redistributable library.\n" +
"Check that you have the latest version installed.\n" +
"See https://learn.microsoft.com/en-us/cpp/windows/latest-supported-vc-redist.";
}
logger.error(error);

// Log the original error to the debug logs.
logger.debug(chunk);
}

// IGNORABLE:
// anything else not handled above is considered ignorable
// so send it to the debug logs which are discarded unless
// the user explicitly sets a logLevel indicating they care
logger.debug(chunk);
else {
logger.debug(chunk);
}
}

// known case: warnings are not errors, log them as such
Expand All @@ -796,7 +817,7 @@ export function handleRuntimeStdio(stdout: Readable, stderr: Readable) {
// ignore entirely, don't even send it to the debug log file
}

// anything not exlicitly handled above should be logged as an error (via stderr)
// anything not explicitly handled above should be logged as an error (via stderr)
else {
logger.error(getSourceMappedString(chunk));
}
Expand Down
2 changes: 1 addition & 1 deletion pnpm-lock.yaml

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 56a3de2

Please sign in to comment.