Skip to content

Commit

Permalink
meta(eslint-config): Don't allow voiding Promises (#9814)
Browse files Browse the repository at this point in the history
  • Loading branch information
lforst authored Dec 20, 2023
1 parent bf32ea4 commit 7ace093
Show file tree
Hide file tree
Showing 40 changed files with 179 additions and 82 deletions.
41 changes: 18 additions & 23 deletions packages/browser-integration-tests/utils/replayHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -105,40 +105,35 @@ export function waitForReplayRequest(
* Wait until a callback returns true, collecting all replay responses along the way.
* This can be useful when you don't know if stuff will be in one or multiple replay requests.
*/
export function waitForReplayRequests(
export async function waitForReplayRequests(
page: Page,
callback: (event: ReplayEvent, res: Response) => boolean,
timeout?: number,
): Promise<Response[]> {
const responses: Response[] = [];

return new Promise<Response[]>(resolve => {
void page.waitForResponse(
res => {
const req = res.request();
await page.waitForResponse(
res => {
const req = res.request();

const event = getReplayEventFromRequest(req);
const event = getReplayEventFromRequest(req);

if (!event) {
return false;
}
if (!event) {
return false;
}

responses.push(res);
responses.push(res);

try {
if (callback(event, res)) {
resolve(responses);
return true;
}
try {
return callback(event, res);
} catch {
return false;
}
},
timeout ? { timeout } : undefined,
);

return false;
} catch {
return false;
}
},
timeout ? { timeout } : undefined,
);
});
return responses;
}

export function isReplayEvent(event: Event): event is ReplayEvent {
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();
}, 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 @@ -79,10 +79,16 @@ function installGlobalErrorHandler(client: Client): void {
data.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;
},
);
});
}

Expand Down Expand Up @@ -122,10 +128,16 @@ function installGlobalUnhandledRejectionHandler(client: Client): 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;
},
);
});
}

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();
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 @@ -88,7 +88,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
8 changes: 6 additions & 2 deletions packages/node/src/integrations/anr/worker.ts
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,9 @@ if (options.captureStackTrace) {
session.post('Debugger.disable');

const context = trace_id?.length && span_id?.length ? { trace_id, span_id, parent_span_id } : undefined;
void sendAnrEvent(stackFrames, context);
sendAnrEvent(stackFrames, context).then(null, () => {
log('Sending ANR event failed.');
});
},
);
} catch (e) {
Expand Down Expand Up @@ -196,7 +198,9 @@ function watchdogTimeout(): void {
debuggerPause();
} else {
log('Capturing event without a stack trace');
void sendAnrEvent();
sendAnrEvent().then(null, () => {
log('Sending ANR event failed on watchdog timeout.');
});
}
}

Expand Down
Loading

0 comments on commit 7ace093

Please sign in to comment.