Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

meta(eslint-config): Don't allow voiding Promises #9814

Merged
merged 12 commits into from
Dec 20, 2023
3 changes: 2 additions & 1 deletion packages/browser-integration-tests/utils/replayHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,8 @@ export function waitForReplayRequests(
const responses: Response[] = [];

return new Promise<Response[]>(resolve => {
void page.waitForResponse(
// eslint-disable-next-line @typescript-eslint/no-floating-promises
page.waitForResponse(
anonrig marked this conversation as resolved.
Show resolved Hide resolved
res => {
const req = res.request();

Expand Down
10 changes: 8 additions & 2 deletions packages/browser/src/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -106,7 +106,10 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
dsn: this.getDsn(),
tunnel: this.getOptions().tunnel,
});
void this._sendEnvelope(envelope);

// _sendEnvelope should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._sendEnvelope(envelope);
}

/**
Expand Down Expand Up @@ -137,6 +140,9 @@ export class BrowserClient extends BaseClient<BrowserClientOptions> {
DEBUG_BUILD && logger.log('Sending outcomes:', outcomes);

const envelope = createClientReportEnvelope(outcomes, this._options.tunnel && dsnToString(this._dsn));
void this._sendEnvelope(envelope);

// _sendEnvelope should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._sendEnvelope(envelope);
}
}
3 changes: 2 additions & 1 deletion packages/browser/src/profiling/hubextensions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,8 @@ export function startProfileForTransaction(transaction: Transaction): Transactio
);
}
// If the timeout exceeds, we want to stop profiling, but not finish the transaction
void onProfileHandler();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
onProfileHandler();
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
}, MAX_PROFILE_DURATION_MS);

// We need to reference the original finish call to avoid creating an infinite loop
Expand Down
10 changes: 8 additions & 2 deletions packages/core/src/baseclient.ts
Original file line number Diff line number Diff line change
Expand Up @@ -375,7 +375,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
*/
public sendSession(session: Session | SessionAggregates): void {
const env = createSessionEnvelope(session, this._dsn, this._options._metadata, this._options.tunnel);
void this._sendEnvelope(env);

// _sendEnvelope should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._sendEnvelope(env);
}

/**
Expand Down Expand Up @@ -409,7 +412,10 @@ export abstract class BaseClient<O extends ClientOptions> implements Client<O> {
this._options._metadata,
this._options.tunnel,
);
void this._sendEnvelope(metricsEnvelope);

// _sendEnvelope should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._sendEnvelope(metricsEnvelope);
}

// Keep on() & emit() signatures in sync with types' client.ts interface
Expand Down
6 changes: 5 additions & 1 deletion packages/core/src/server-runtime-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,11 @@ export class ServerRuntimeClient<
);

DEBUG_BUILD && logger.info('Sending checkin:', checkIn.monitorSlug, checkIn.status);
void this._sendEnvelope(envelope);

// _sendEnvelope should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._sendEnvelope(envelope);

return id;
}

Expand Down
28 changes: 20 additions & 8 deletions packages/deno/src/integrations/globalhandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,16 @@ function installGlobalErrorHandler(): void {
data.preventDefault();
isExiting = true;

void flush().then(() => {
// rethrow to replicate Deno default behavior
throw error;
});
void flush().then(
anonrig marked this conversation as resolved.
Show resolved Hide resolved
() => {
// rethrow to replicate Deno default behavior
throw error;
},
() => {
// rethrow to replicate Deno default behavior
throw error;
anonrig marked this conversation as resolved.
Show resolved Hide resolved
},
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
);
});
}

Expand Down Expand Up @@ -130,10 +136,16 @@ function installGlobalUnhandledRejectionHandler(): void {
e.preventDefault();
isExiting = true;

void flush().then(() => {
// rethrow to replicate Deno default behavior
throw error;
});
flush().then(
() => {
// rethrow to replicate Deno default behavior
throw error;
},
() => {
// rethrow to replicate Deno default behavior
throw error;
},
Lms24 marked this conversation as resolved.
Show resolved Hide resolved
);
});
}

Expand Down
3 changes: 2 additions & 1 deletion packages/e2e-tests/prepare.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,5 @@ async function run(): Promise<void> {
}
}

void run();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
Copy link
Contributor

Choose a reason for hiding this comment

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

Wdyt?

Suggested change
run();
(async () => { await run() })();

Copy link
Member Author

Choose a reason for hiding this comment

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

This will still create a floating promise

3 changes: 2 additions & 1 deletion packages/e2e-tests/run.ts
Original file line number Diff line number Diff line change
Expand Up @@ -80,4 +80,5 @@ async function run(): Promise<void> {
}
}

void run();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
15 changes: 13 additions & 2 deletions packages/eslint-config-sdk/src/base.js
Original file line number Diff line number Diff line change
Expand Up @@ -84,7 +84,7 @@ module.exports = {
'@typescript-eslint/no-unused-expressions': ['error', { allowShortCircuit: true }],

// Make sure Promises are handled appropriately
'@typescript-eslint/no-floating-promises': 'error',
'@typescript-eslint/no-floating-promises': ['error', { ignoreVoid: false }],

// Disallow delete operator. We should make this operation opt in (by disabling this rule).
'@typescript-eslint/no-dynamic-delete': 'error',
Expand Down Expand Up @@ -158,7 +158,17 @@ module.exports = {
env: {
jest: true,
},
files: ['test.ts', '*.test.ts', '*.test.tsx', '*.test.js', '*.test.jsx', 'test/**/*.ts', 'test/**/*.js'],
files: [
'test.ts',
'*.test.ts',
'*.test.tsx',
'*.test.js',
'*.test.jsx',
'test/**/*.ts',
'test/**/*.js',
'tests/**/*.ts',
'tests/**/*.js',
],
rules: {
'max-lines': 'off',
'@typescript-eslint/explicit-function-return-type': 'off',
Expand All @@ -171,6 +181,7 @@ module.exports = {
'@typescript-eslint/no-empty-function': 'off',
'@sentry-internal/sdk/no-optional-chaining': 'off',
'@sentry-internal/sdk/no-nullish-coalescing': 'off',
'@typescript-eslint/no-floating-promises': 'off',
},
},
{
Expand Down
3 changes: 2 additions & 1 deletion packages/integrations/scripts/buildBundles.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,8 @@ if (runParallel) {
process.exit(1);
});
} else {
void (async () => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
(async () => {
for (const integration of getIntegrations()) {
await buildBundle(integration, 'es5');
await buildBundle(integration, 'es6');
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/common/utils/responseEnd.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import type { ResponseEndMethod, WrappedResponseEndMethod } from '../types';
export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: ServerResponse): void {
const wrapEndMethod = (origEnd: ResponseEndMethod): WrappedResponseEndMethod => {
return function sentryWrappedEnd(this: ServerResponse, ...args: unknown[]) {
void finishTransaction(transaction, this);
finishTransaction(transaction, this);
return origEnd.call(this, ...args);
};
};
Expand All @@ -39,7 +39,7 @@ export function autoEndTransactionOnResponseEnd(transaction: Transaction, res: S
}

/** Finish the given response's transaction and set HTTP status data */
export async function finishTransaction(transaction: Transaction | undefined, res: ServerResponse): Promise<void> {
export function finishTransaction(transaction: Transaction | undefined, res: ServerResponse): void {
if (transaction) {
transaction.setHttpStatus(res.statusCode);
transaction.finish();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,9 @@ async function withServerActionInstrumentationImplementation<A extends (...args:
}

if (process.env.NEXT_RUNTIME === 'edge') {
void flushQueue();
// flushQueue should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
flushQueue();
}
}

Expand Down
7 changes: 3 additions & 4 deletions packages/nextjs/src/common/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
const origResEnd = res.end;
res.end = async function (this: unknown, ...args: unknown[]) {
if (transaction) {
await finishTransaction(transaction, res);
finishTransaction(transaction, res);
await flushQueue();
}

Expand Down Expand Up @@ -207,15 +207,14 @@ export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: stri
res.statusCode = 500;
res.statusMessage = 'Internal Server Error';

finishTransaction(transaction, res);

// Make sure we have a chance to finish the transaction and flush events to Sentry before the handler errors
// out. (Apps which are deployed on Vercel run their API routes in lambdas, and those lambdas will shut down the
// moment they detect an error, so it's important to get this done before rethrowing the error. Apps not
// deployed serverlessly will run into this cleanup code again in `res.end(), but the transaction will already
// be finished and the queue will already be empty, so effectively it'll just no-op.)
if (platformSupportsStreaming() && !wrappingTarget.__sentry_test_doesnt_support_streaming__) {
void finishTransaction(transaction, res);
} else {
await finishTransaction(transaction, res);
await flushQueue();
}

Expand Down
4 changes: 3 additions & 1 deletion packages/nextjs/src/common/wrapServerComponentWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,9 @@ export function wrapServerComponentWithSentry<F extends (...args: any[]) => any>
}
},
() => {
void flushQueue();
// flushQueue should not throw
// eslint-disable-next-line @typescript-eslint/no-floating-promises
flushQueue();
},
);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,8 @@ const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'tra

Sentry.getCurrentScope().setSpan(transaction);

void (async () => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
(async () => {
// Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation
await server.executeOperation({
query: '{hello}',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,4 +41,5 @@ async function run(): Promise<void> {
}
}

void run();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@ async function run(): Promise<void> {
}
}

void run();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ const transaction = Sentry.startTransaction({ name: 'test_transaction', op: 'tra

Sentry.getCurrentScope().setSpan(transaction);

void (async () => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
(async () => {
// Ref: https://www.apollographql.com/docs/apollo-server/testing/testing/#testing-using-executeoperation
await server.executeOperation({
query: '{hello}',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,4 +42,5 @@ async function run(): Promise<void> {
}
}

void run();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
Original file line number Diff line number Diff line change
Expand Up @@ -44,4 +44,5 @@ async function run(): Promise<void> {
}
}

void run();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
run();
6 changes: 4 additions & 2 deletions packages/node-integration-tests/utils/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -184,7 +184,8 @@ export class TestEnv {
envelopeTypeArray,
);

void makeRequest(options.method, options.url || this.url, this._axiosConfig);
// eslint-disable-next-line @typescript-eslint/no-floating-promises
makeRequest(options.method, options.url || this.url, this._axiosConfig);
return resProm;
}

Expand Down Expand Up @@ -305,7 +306,8 @@ export class TestEnv {

nock.cleanAll();

void this._closeServer().then(() => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
this._closeServer().then(() => {
resolve(reqCount);
});
},
Expand Down
3 changes: 2 additions & 1 deletion packages/node-integration-tests/utils/run-tests.ts
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,8 @@ const workers = os.cpus().map(async (_, i) => {
}
});

void Promise.all(workers).then(() => {
// eslint-disable-next-line @typescript-eslint/no-floating-promises
Promise.all(workers).then(() => {
console.log('-------------------');
console.log(`Successfully ran ${numTests} tests.`);
if (fails.length > 0) {
Expand Down
14 changes: 10 additions & 4 deletions packages/node/src/anr/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,10 +183,16 @@ function handleChildProcess(options: Options): void {

captureEvent(createAnrEvent(options.anrThreshold, frames));

void flush(3000).then(() => {
// We only capture one event to avoid spamming users with errors
process.exit();
});
flush(3000).then(
() => {
// We only capture one event to avoid spamming users with errors
process.exit();
},
() => {
// Flush can reject and we want to fail gracefully
process.exit();
},
);
}

addEventProcessor(event => {
Expand Down
6 changes: 4 additions & 2 deletions packages/overhead-metrics/src/perf/network.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,11 +71,13 @@ export class NetworkUsageCollector {
this._events.push(event);
// Note: playwright would error out on file:/// requests. They are used to access local test app resources.
if (url.startsWith('file:///')) {
void route.continue();
// eslint-disable-next-line @typescript-eslint/no-floating-promises
route.continue();
} else {
const response = await route.fetch();
const body = await response.body();
void route.fulfill({ response, body });
// eslint-disable-next-line @typescript-eslint/no-floating-promises
route.fulfill({ response, body });
event.responseTimeNs = process.hrtime.bigint();
event.responseSize = body.length;
}
Expand Down
4 changes: 3 additions & 1 deletion packages/remix/src/utils/instrumentServer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,9 @@ export function wrapRemixHandleError(err: unknown, { request }: DataFunctionArgs
return;
}

void captureRemixServerException(err, 'remix.server.handleError', request);
captureRemixServerException(err, 'remix.server.handleError', request).then(null, e => {
DEBUG_BUILD && logger.warn('Failed to capture Remix Server exception.', e);
});
}

/**
Expand Down
4 changes: 3 additions & 1 deletion packages/replay/src/coreHandlers/handleAfterSendEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -74,7 +74,9 @@ function handleErrorEvent(replay: ReplayContainer, event: ErrorEvent): void {

setTimeout(() => {
// Capture current event buffer as new replay
void replay.sendBufferedReplayOrFlush();
// This should never reject
// eslint-disable-next-line @typescript-eslint/no-floating-promises
replay.sendBufferedReplayOrFlush();
});
}

Expand Down
4 changes: 3 additions & 1 deletion packages/replay/src/coreHandlers/handleGlobalEvent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,9 @@ export function handleGlobalEventListener(
}

if (isFeedbackEvent(event)) {
void replay.flush();
// This should never reject
// eslint-disable-next-line @typescript-eslint/no-floating-promises
replay.flush();
event.contexts.feedback.replay_id = replay.getSessionId();
// Add a replay breadcrumb for this piece of feedback
addFeedbackBreadcrumb(replay, event);
Expand Down
Loading