Skip to content

Commit

Permalink
Rename reportError to logError to avoid new global type
Browse files Browse the repository at this point in the history
There's a relatively recent addition to the DOM global types, due to a
real method available for web workers, which means it's very very easy
to accidentally call reportError without importing it, which fails
everywhere except web workers, where it merely does completely the wrong
thing.

logError is exactly the same, but without that problem.
  • Loading branch information
pimterry committed Jul 12, 2023
1 parent 42987f2 commit e6f3b7e
Show file tree
Hide file tree
Showing 20 changed files with 69 additions and 69 deletions.
10 changes: 5 additions & 5 deletions src/api/api-model.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { getSystemProxy } from 'os-proxy-config';

import { SERVER_VERSION } from "../constants";
import { delay } from '../util/promise';
import { reportError, addBreadcrumb } from '../error-tracking';
import { logError, addBreadcrumb } from '../error-tracking';
import { HtkConfig } from "../config";
import { ActivationError, Interceptor } from "../interceptors";
import { getDnsServer } from '../dns-server';
Expand Down Expand Up @@ -170,7 +170,7 @@ export class ApiModel {
// After 30s, don't stop activating, but report an error if we're not done yet
let activationDone = false;
delay(30000).then(() => {
if (!activationDone) reportError(`Timeout activating ${id}`)
if (!activationDone) logError(`Timeout activating ${id}`)
});

try {
Expand All @@ -183,7 +183,7 @@ export class ApiModel {
activationDone = true;
if (activationError.reportable !== false) {
addBreadcrumb(`Failed to activate ${id}`, { category: 'interceptor' });
reportError(err);
logError(err);
}
return {
success: false,
Expand All @@ -202,7 +202,7 @@ export class ApiModel {
const interceptor = this.interceptors[id];
if (!interceptor) throw new Error(`Unknown interceptor ${id}`);

await interceptor.deactivate(proxyPort, options).catch(reportError);
await interceptor.deactivate(proxyPort, options).catch(logError);
return { success: !interceptor.isActive(proxyPort) };
}

Expand All @@ -219,7 +219,7 @@ export class ApiModel {
const withFallback = <R>(p: () => Promise<R>, timeoutMs: number, defaultValue: R) =>
Promise.race([
p().catch((error) => {
reportError(error);
logError(error);
return defaultValue;
}),
delay(timeoutMs).then(() => defaultValue)
Expand Down
4 changes: 2 additions & 2 deletions src/api/rest-api.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import type {
import type { ParsedQs } from 'qs';

import { ErrorLike, StatusError } from '../util/error';
import { reportError } from '../error-tracking';
import { logError } from '../error-tracking';
import { ApiModel } from './api-model';
import * as Client from '../client/client-types';

Expand Down Expand Up @@ -175,7 +175,7 @@ function handleErrors<
const error = e as ErrorLike;

console.log(`Error handling request to ${req.path}: ${error.message ?? error}`);
reportError(error);
logError(error);

// Use default error handler if response started (kills the connection)
if (res.headersSent) return next(error)
Expand Down
10 changes: 5 additions & 5 deletions src/browsers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import {
update as updateBrowserCacheCb
} from '@httptoolkit/browser-launcher';

import { reportError } from './error-tracking';
import { logError } from './error-tracking';
import { delay } from './util/promise';
import { isErrorLike } from './util/error';
import { readFile, deleteFile } from './util/fs';
Expand Down Expand Up @@ -40,7 +40,7 @@ export async function checkBrowserConfig(configPath: string) {
if (isErrorLike(err) && err.code === 'ENOENT') return;

console.error('Failed to clear broken config file:', err);
reportError(err);
logError(err);
});
}
}
Expand All @@ -63,13 +63,13 @@ function getLauncher(configPath: string) {
// Need to reload the launcher after updating the cache:
launcher = getBrowserLauncher(browserConfig);
} catch (e) {
reportError(e)
logError(e)
}
});

// Reset & retry if this fails somehow:
launcher.catch((e) => {
reportError(e);
logError(e);
launcher = undefined;
});
}
Expand All @@ -92,7 +92,7 @@ export const launchBrowser = async (url: string, options: LaunchOptions, configP
// fallback error handling: log & report & don't crash.
if (browserInstance.process.listenerCount('error') === 1) {
console.log('Browser launch error');
reportError(e);
logError(e);
}
});

Expand Down
18 changes: 9 additions & 9 deletions src/commands/start.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ function maybeBundleImport<T>(moduleName: string): T {
return require('../' + moduleName);
}
}
const { initErrorTracking, reportError } = maybeBundleImport<ErrorTrackingModule>('error-tracking');
const { initErrorTracking, logError } = maybeBundleImport<ErrorTrackingModule>('error-tracking');
initErrorTracking();

import { Command, flags } from '@oclif/command'
Expand All @@ -49,7 +49,7 @@ class HttpToolkitServer extends Command {
configPath: flags.config,
authToken: envToken || flags.token
}).catch(async (error) => {
await reportError(error);
await logError(error);
throw error;
});
}
Expand All @@ -65,7 +65,7 @@ class HttpToolkitServer extends Command {

// Be careful - if the server path isn't clearly ours somehow, ignore it.
if (!isOwnedPath(serverUpdatesPath)) {
reportError(`Unexpected server updates path (${serverUpdatesPath}), ignoring`);
logError(`Unexpected server updates path (${serverUpdatesPath}), ignoring`);
return;
}

Expand All @@ -84,19 +84,19 @@ class HttpToolkitServer extends Command {
filename !== '.DS_Store' // Meaningless Mac folder metadata
)) {
console.log(serverPaths);
reportError(
logError(
`Server path (${serverUpdatesPath}) contains unexpected content, ignoring`
);
return;
}

const maybeReportError = (error: Error & { code?: string }) => {
const maybeLogError = (error: Error & { code?: string }) => {
if ([
'EBUSY',
'EPERM'
].includes(error.code!)) return;

else reportError(error);
else logError(error);
};

if (serverPaths.every((filename) => {
Expand All @@ -107,7 +107,7 @@ class HttpToolkitServer extends Command {
// a new server standalone (not just from an update), because otherwise the
// update dir can end up in a broken state. Better to clear it completely.
console.log("Downloaded server directory is entirely outdated, deleting it");
deleteFolder(serverUpdatesPath).catch(maybeReportError);
deleteFolder(serverUpdatesPath).catch(maybeLogError);
} else {
// Some of the servers are outdated, but not all (maybe it includes us).
// Async delete all server versions older than this currently running version.
Expand All @@ -116,7 +116,7 @@ class HttpToolkitServer extends Command {

if (version && semver.lt(version, currentVersion)) {
console.log(`Deleting old server ${filename}`);
deleteFolder(path.join(serverUpdatesPath, filename)).catch(maybeReportError);
deleteFolder(path.join(serverUpdatesPath, filename)).catch(maybeLogError);
}
});
}
Expand Down Expand Up @@ -151,7 +151,7 @@ function isOwnedPath(input: string) {
if (input.split(path.sep).includes('httptoolkit-server')) {
return true;
} else {
reportError(`Unexpected unowned path ${input}`);
logError(`Unexpected unowned path ${input}`);
return false;
}
}
Expand Down
2 changes: 1 addition & 1 deletion src/error-tracking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ export function addBreadcrumb(message: string, data: Sentry.Breadcrumb) {
Sentry.addBreadcrumb(Object.assign({ message }, data));
}

export function reportError(error: Error | string | unknown): undefined | Promise<void> {
export function logError(error: Error | string | unknown): undefined | Promise<void> {
console.warn(error);
if (!sentryInitialized) return;

Expand Down
4 changes: 2 additions & 2 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import updateCommand from '@oclif/plugin-update/lib/commands/update';

import { HttpToolkitServerApi } from './api/api-server';
import { checkBrowserConfig } from './browsers';
import { reportError } from './error-tracking';
import { logError } from './error-tracking';
import { MOCKTTP_ALLOWED_ORIGINS } from './constants';

import { delay } from './util/promise';
Expand Down Expand Up @@ -207,7 +207,7 @@ export async function runHTK(options: {
}

console.log(error);
reportError('Failed to check for updates');
logError('Failed to check for updates');
})
);
});
Expand Down
8 changes: 4 additions & 4 deletions src/interceptors/android/adb-commands.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import * as stream from 'stream';
import * as path from 'path';
import adb, * as Adb from '@devicefarmer/adbkit';
import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';
import { isErrorLike } from '../../util/error';
import { delay, waitUntil } from '../../util/promise';
import { getCertificateFingerprint, parseCert } from '../../certificates';
Expand Down Expand Up @@ -35,7 +35,7 @@ export function createAdbClient() {

// We listen for errors and report them. This only happens if adbkit completely
// fails to handle or listen to a connection error. We'd rather report that than crash.
client.on('error', reportError);
client.on('error', logError);

return client;
}
Expand Down Expand Up @@ -92,7 +92,7 @@ export const getConnectedDevices = batchCalls(async (adbClient: Adb.Client) => {
}
return [];
} else {
reportError(e);
logError(e);
throw e;
}
}
Expand Down Expand Up @@ -222,7 +222,7 @@ export async function getRootCommand(adbClient: Adb.DeviceClient): Promise<RootC
: undefined; // Still not root, no luck.
} catch (e) {
console.error(e);
reportError('ADB root check crashed');
logError('ADB root check crashed');
return undefined;
} finally {
// Try to clean up the root test script, just to be tidy
Expand Down
6 changes: 3 additions & 3 deletions src/interceptors/android/android-adb-interceptor.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import { Interceptor } from '..';
import { HtkConfig } from '../../config';
import { generateSPKIFingerprint } from 'mockttp';

import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';
import { delay } from '../../util/promise';
import { isErrorLike } from '../../util/error';
import {
Expand Down Expand Up @@ -89,7 +89,7 @@ export class AndroidAdbInterceptor implements Interceptor {
await bringToFront(
deviceClient,
'tech.httptoolkit.android.v1/tech.httptoolkit.android.MainActivity'
).catch(reportError); // Not that important, so we continue if this fails somehow
).catch(logError); // Not that important, so we continue if this fails somehow

// Build a trigger URL to activate the proxy on the device:
const setupParams = {
Expand Down Expand Up @@ -225,7 +225,7 @@ export class AndroidAdbInterceptor implements Interceptor {
]);
console.log('Android Chrome flags set');
} catch (e) {
reportError(e);
logError(e);
}
}
}
8 changes: 4 additions & 4 deletions src/interceptors/android/fetch-apk.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import fetch from 'node-fetch';

import { readDir, createTmp, moveFile, deleteFile } from '../../util/fs';
import { HtkConfig } from '../../config';
import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';

async function getLatestRelease(): Promise<{ version: string, url: string } | undefined> {
try {
Expand Down Expand Up @@ -53,7 +53,7 @@ async function getLatestLocalApk(config: HtkConfig) {
else return latestLocalApk;
} catch (e) {
console.log("Could not check for local Android app APK", e);
reportError(e);
logError(e);
}
}

Expand Down Expand Up @@ -131,7 +131,7 @@ export async function streamLatestApk(config: HtkConfig): Promise<stream.Readabl
const apkOutputStream = new stream.PassThrough({ highWaterMark: 10485760 });
apkStream.pipe(apkOutputStream);

updateLocalApk(latestApkRelease.version, apkFileStream, config).catch(reportError);
updateLocalApk(latestApkRelease.version, apkFileStream, config).catch(logError);
return apkOutputStream;
}
}
Expand All @@ -147,7 +147,7 @@ export async function streamLatestApk(config: HtkConfig): Promise<stream.Readabl
fetch(latestApkRelease.url).then((apkResponse) => {
const apkStream = apkResponse.body;
return updateLocalApk(latestApkRelease.version, apkStream, config);
}).catch(reportError);
}).catch(logError);

console.log('Streaming local APK, and updating it async');
return fs.createReadStream(localApk.path);
Expand Down
4 changes: 2 additions & 2 deletions src/interceptors/chromium-based-interceptors.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import { readFile, deleteFolder } from '../util/fs';
import { listRunningProcesses, windowsClose, waitForExit } from '../util/process-management';
import { HideWarningServer } from '../hide-warning-server';
import { Interceptor } from '.';
import { reportError } from '../error-tracking';
import { logError } from '../error-tracking';
import { WEBEXTENSION_INSTALL } from '../webextension';

const getBrowserDetails = async (config: HtkConfig, variant: string): Promise<Browser | undefined> => {
Expand Down Expand Up @@ -135,7 +135,7 @@ abstract class FreshChromiumBasedInterceptor implements Interceptor {

const profilePath = browserDetails.profile;
if (!profilePath.startsWith(this.config.configPath)) {
reportError(
logError(
`Unexpected ${this.variantName} profile location, not deleting: ${profilePath}`
);
} else {
Expand Down
4 changes: 2 additions & 2 deletions src/interceptors/docker/docker-interception-services.ts
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import Docker from 'dockerode';
import { ProxySettingCallback } from 'mockttp';

import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';
import { addShutdownHandler } from '../../shutdown';

import { DOCKER_BUILD_LABEL } from './docker-build-injection';
Expand Down Expand Up @@ -117,7 +117,7 @@ export async function ensureDockerServicesRunning(proxyPort: number) {
getDnsServer(proxyPort),
// We don't double-check on the injection volume here - that's
// checked separately at the point of use instead.
]).catch(reportError);
]).catch(logError);
}

export async function stopDockerInterceptionServices(
Expand Down
4 changes: 2 additions & 2 deletions src/interceptors/docker/docker-networking.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@ import Docker from 'dockerode';
import * as EventStream from 'event-stream';
import * as mobx from 'mobx';

import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';

import { isInterceptedContainer } from './docker-commands';
import { isDockerAvailable } from './docker-interception-services';
Expand Down Expand Up @@ -96,7 +96,7 @@ export async function monitorDockerNetworkAliases(proxyPort: number): Promise<Do
const stream = getDockerEventStream(docker);
stream.on('error', (e) => {
console.log(`Docker stream for port ${proxyPort} hit an error`);
reportError(e);
logError(e);
});

const dnsServer = await getDnsServer(proxyPort);
Expand Down
4 changes: 2 additions & 2 deletions src/interceptors/docker/docker-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { makeDestroyable, DestroyableServer } from 'destroyable-server';
import { chmod, deleteFile, readDir } from '../../util/fs';
import { rawHeadersToHeaders } from '../../util/http';
import { streamToBuffer } from '../../util/stream';
import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';
import { addShutdownHandler } from '../../shutdown';

import {
Expand Down Expand Up @@ -153,7 +153,7 @@ async function createDockerProxy(
if (reqPath.match(BUILD_IMAGE_MATCHER)) {
if (reqUrl.searchParams.get('remote')) {
res.writeHead(400);
reportError("Build interception failed due to unsupported 'remote' param");
logError("Build interception failed due to unsupported 'remote' param");

if (reqUrl.searchParams.get('remote') === 'client-session') {
res.end("HTTP Toolkit does not yet support BuildKit-powered builds");
Expand Down
4 changes: 2 additions & 2 deletions src/interceptors/docker/docker-tunnel-proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import { Mutex } from 'async-mutex';
import { isImageAvailable } from './docker-commands';
import { isDockerAvailable } from './docker-interception-services';
import { delay } from '../../util/promise';
import { reportError } from '../../error-tracking';
import { logError } from '../../error-tracking';
import { waitForDockerStream } from './docker-utils';

const DOCKER_TUNNEL_IMAGE = "httptoolkit/docker-socks-tunnel:v1.2.0";
Expand Down Expand Up @@ -100,7 +100,7 @@ export function ensureDockerTunnelRunning(proxyPort: number) {
})
);
portCache[proxyPort] = refreshTunnelPort;
refreshTunnelPort.catch(reportError);
refreshTunnelPort.catch(logError);
}
}).finally(() => {
// Clean up the promise, so that future calls to ensureRunning re-run this check.
Expand Down
Loading

0 comments on commit e6f3b7e

Please sign in to comment.