Skip to content

Commit

Permalink
feat(eslint): Add no-return-await rule to eslint config (#8388)
Browse files Browse the repository at this point in the history
Add https://eslint.org/docs/latest/rules/no-return-await to eslint
config to remove usage of uneeded async/await calls. This helps reduce
microtasks being generated, which can help reduce memory pressure caused
by the SDK.

The downside of removing `return await` is that stacktraces get slightly
worse for async errors that use these methods, as we no longer pause
execution on return for the engine to grab context on, but instead just
pass through the promise, but I think it's worth it for this to be the
default, and for us to opt-in to the better stacktraces if need be.
  • Loading branch information
AbhiPrasad authored Jun 23, 2023
1 parent 5217485 commit a34581d
Show file tree
Hide file tree
Showing 20 changed files with 35 additions and 32 deletions.
2 changes: 1 addition & 1 deletion packages/browser-integration-tests/utils/helpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -268,7 +268,7 @@ async function injectScriptAndGetEvents(page: Page, url: string, scriptPath: str
await page.goto(url);
await runScriptInSandbox(page, scriptPath);

return await getSentryEvents(page);
return getSentryEvents(page);
}

export {
Expand Down
4 changes: 2 additions & 2 deletions packages/browser-integration-tests/utils/replayHelpers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -132,14 +132,14 @@ export async function waitForReplayRunning(page: Page): Promise<void> {
* Note that due to how this works with playwright, this is a POJO copy of replay.
* This means that we cannot access any methods on it, and also not mutate it in any way.
*/
export async function getReplaySnapshot(page: Page): Promise<{
export function getReplaySnapshot(page: Page): Promise<{
_isPaused: boolean;
_isEnabled: boolean;
_context: InternalEventContext;
session: Session | undefined;
recordingMode: ReplayRecordingMode;
}> {
return await page.evaluate(() => {
return page.evaluate(() => {
const replayIntegration = (window as unknown as Window & { Replay: { _replay: ReplayContainer } }).Replay;
const replay = replayIntegration._replay;

Expand Down
4 changes: 2 additions & 2 deletions packages/e2e-tests/test-utils/event-proxy-server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,7 @@ async function registerCallbackServerPort(serverName: string, port: string): Pro
await writeFile(tmpFilePath, port, { encoding: 'utf8' });
}

async function retrieveCallbackServerPort(serverName: string): Promise<string> {
function retrieveCallbackServerPort(serverName: string): Promise<string> {
const tmpFilePath = path.join(os.tmpdir(), `${TEMP_FILE_PREFIX}${serverName}`);
return await readFile(tmpFilePath, 'utf8');
return readFile(tmpFilePath, 'utf8');
}
3 changes: 3 additions & 0 deletions packages/eslint-config-sdk/src/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -261,5 +261,8 @@ module.exports = {
'array-callback-return': ['error', { allowImplicit: true }],

quotes: ['error', 'single', { avoidEscape: true }],

// Remove uncessary usages of async await to prevent extra micro-tasks
'no-return-await': 'error',
},
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ type AppGetInitialProps = (typeof App)['getInitialProps'];
*/
export function wrapAppGetInitialPropsWithSentry(origAppGetInitialProps: AppGetInitialProps): AppGetInitialProps {
return new Proxy(origAppGetInitialProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<AppGetInitialProps>) => {
return await wrappingTarget.apply(thisArg, args);
apply: (wrappingTarget, thisArg, args: Parameters<AppGetInitialProps>) => {
return wrappingTarget.apply(thisArg, args);
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,8 @@ export function wrapDocumentGetInitialPropsWithSentry(
origDocumentGetInitialProps: DocumentGetInitialProps,
): DocumentGetInitialProps {
return new Proxy(origDocumentGetInitialProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<DocumentGetInitialProps>) => {
return await wrappingTarget.apply(thisArg, args);
apply: (wrappingTarget, thisArg, args: Parameters<DocumentGetInitialProps>) => {
return wrappingTarget.apply(thisArg, args);
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ export function wrapErrorGetInitialPropsWithSentry(
origErrorGetInitialProps: ErrorGetInitialProps,
): ErrorGetInitialProps {
return new Proxy(origErrorGetInitialProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<ErrorGetInitialProps>) => {
return await wrappingTarget.apply(thisArg, args);
apply: (wrappingTarget, thisArg, args: Parameters<ErrorGetInitialProps>) => {
return wrappingTarget.apply(thisArg, args);
},
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/client/wrapGetInitialPropsWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ type GetInitialProps = Required<NextPage>['getInitialProps'];
*/
export function wrapGetInitialPropsWithSentry(origGetInitialProps: GetInitialProps): GetInitialProps {
return new Proxy(origGetInitialProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<GetInitialProps>) => {
return await wrappingTarget.apply(thisArg, args);
apply: (wrappingTarget, thisArg, args: Parameters<GetInitialProps>) => {
return wrappingTarget.apply(thisArg, args);
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,8 @@ import type { GetServerSideProps } from 'next';
*/
export function wrapGetServerSidePropsWithSentry(origGetServerSideProps: GetServerSideProps): GetServerSideProps {
return new Proxy(origGetServerSideProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<GetServerSideProps>) => {
return await wrappingTarget.apply(thisArg, args);
apply: (wrappingTarget, thisArg, args: Parameters<GetServerSideProps>) => {
return wrappingTarget.apply(thisArg, args);
},
});
}
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/client/wrapGetStaticPropsWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ type Props = { [key: string]: unknown };
*/
export function wrapGetStaticPropsWithSentry(origGetStaticProps: GetStaticProps<Props>): GetStaticProps<Props> {
return new Proxy(origGetStaticProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<GetStaticProps<Props>>) => {
return await wrappingTarget.apply(thisArg, args);
apply: (wrappingTarget, thisArg, args: Parameters<GetStaticProps<Props>>) => {
return wrappingTarget.apply(thisArg, args);
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ export async function devErrorSymbolicationEventProcessor(event: Event, hint: Ev
const frames = stackTraceParser.parse(hint.originalException.stack);

const resolvedFrames = await Promise.all(
frames.map(async frame => await resolveStackFrame(frame, hint.originalException as Error)),
frames.map(frame => resolveStackFrame(frame, hint.originalException as Error)),
);

if (event.exception?.values?.[0].stacktrace?.frames) {
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/edge/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function wrapApiHandlerWithSentry<H extends EdgeRouteHandler>(
parameterizedRoute: string,
): (...params: Parameters<H>) => Promise<ReturnType<H>> {
return new Proxy(handler, {
apply: async (wrappingTarget, thisArg, args: Parameters<H>) => {
apply: (wrappingTarget, thisArg, args: Parameters<H>) => {
const req = args[0];

const activeSpan = !!getCurrentHub().getScope()?.getSpan();
Expand All @@ -25,7 +25,7 @@ export function wrapApiHandlerWithSentry<H extends EdgeRouteHandler>(
mechanismFunctionName: 'wrapApiHandlerWithSentry',
});

return await wrappedHandler.apply(thisArg, args);
return wrappedHandler.apply(thisArg, args);
},
});
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/src/edge/wrapMiddlewareWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export function wrapMiddlewareWithSentry<H extends EdgeRouteHandler>(
middleware: H,
): (...params: Parameters<H>) => Promise<ReturnType<H>> {
return new Proxy(middleware, {
apply: async (wrappingTarget, thisArg, args: Parameters<H>) => {
apply: (wrappingTarget, thisArg, args: Parameters<H>) => {
return withEdgeWrapping(wrappingTarget, {
spanDescription: 'middleware',
spanOp: 'middleware.nextjs',
Expand Down
4 changes: 2 additions & 2 deletions packages/nextjs/src/server/wrapApiHandlerWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ import { autoEndTransactionOnResponseEnd, finishTransaction, flushQueue } from '
*/
export function wrapApiHandlerWithSentry(apiHandler: NextApiHandler, parameterizedRoute: string): NextApiHandler {
return new Proxy(apiHandler, {
apply: async (wrappingTarget, thisArg, args: Parameters<NextApiHandler>) => {
apply: (wrappingTarget, thisArg, args: Parameters<NextApiHandler>) => {
// eslint-disable-next-line deprecation/deprecation
return withSentry(wrappingTarget, parameterizedRoute).apply(thisArg, args);
},
Expand All @@ -49,7 +49,7 @@ export const withSentryAPI = wrapApiHandlerWithSentry;
*/
export function withSentry(apiHandler: NextApiHandler, parameterizedRoute?: string): NextApiHandler {
return new Proxy(apiHandler, {
apply: async (wrappingTarget, thisArg, args: [AugmentedNextApiRequest, AugmentedNextApiResponse]) => {
apply: (wrappingTarget, thisArg, args: [AugmentedNextApiRequest, AugmentedNextApiResponse]) => {
const [req, res] = args;

// We're now auto-wrapping API route handlers using `wrapApiHandlerWithSentry` (which uses `withSentry` under the hood), but
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function wrapDocumentGetInitialPropsWithSentry(
origDocumentGetInitialProps: DocumentGetInitialProps,
): DocumentGetInitialProps {
return new Proxy(origDocumentGetInitialProps, {
apply: async (wrappingTarget, thisArg, args: Parameters<DocumentGetInitialProps>) => {
apply: (wrappingTarget, thisArg, args: Parameters<DocumentGetInitialProps>) => {
if (isBuild()) {
return wrappingTarget.apply(thisArg, args);
}
Expand All @@ -41,7 +41,7 @@ export function wrapDocumentGetInitialPropsWithSentry(
dataFetchingMethodName: 'getInitialProps',
});

return await tracedGetInitialProps.apply(thisArg, args);
return tracedGetInitialProps.apply(thisArg, args);
} else {
return errorWrappedGetInitialProps.apply(thisArg, args);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/nextjs/src/server/wrapGetStaticPropsWithSentry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ export function wrapGetStaticPropsWithSentry(
parameterizedRoute: string,
): GetStaticProps<Props> {
return new Proxy(origGetStaticPropsa, {
apply: async (wrappingTarget, thisArg, args: Parameters<GetStaticProps<Props>>) => {
apply: (wrappingTarget, thisArg, args: Parameters<GetStaticProps<Props>>) => {
if (isBuild()) {
return wrappingTarget.apply(thisArg, args);
}
Expand Down
2 changes: 1 addition & 1 deletion packages/replay/src/util/sendReplay.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ export async function sendReplay(
// will retry in intervals of 5, 10, 30
retryConfig.interval *= ++retryConfig.count;

return await new Promise((resolve, reject) => {
return new Promise((resolve, reject) => {
setTimeout(async () => {
try {
await sendReplay(replayData, retryConfig);
Expand Down
4 changes: 2 additions & 2 deletions packages/replay/test/integration/flush.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,8 +135,8 @@ describe('Integration | flush', () => {

it('long first flush enqueues following events', async () => {
// Mock this to resolve after 20 seconds so that we can queue up following flushes
mockAddPerformanceEntries.mockImplementationOnce(async () => {
return await new Promise(resolve => setTimeout(resolve, 20000));
mockAddPerformanceEntries.mockImplementationOnce(() => {
return new Promise(resolve => setTimeout(resolve, 20000));
});

expect(mockAddPerformanceEntries).not.toHaveBeenCalled();
Expand Down
2 changes: 1 addition & 1 deletion packages/sveltekit/src/vite/svelteConfig.ts
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ export function getHooksFileName(svelteConfig: Config, hookType: 'client' | 'ser
*/
export async function getAdapterOutputDir(svelteConfig: Config, adapter: SupportedSvelteKitAdapters): Promise<string> {
if (adapter === 'node') {
return await getNodeAdapterOutputDir(svelteConfig);
return getNodeAdapterOutputDir(svelteConfig);
}

// Auto and Vercel adapters simply use config.kit.outDir
Expand Down
4 changes: 2 additions & 2 deletions packages/utils/test/buildPolyfills/originals.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,11 @@
// the modified versions do the same thing the originals do.

// From Sucrase
export async function _asyncNullishCoalesce(lhs, rhsFn) {
export function _asyncNullishCoalesce(lhs, rhsFn) {
if (lhs != null) {
return lhs;
} else {
return await rhsFn();
return rhsFn();
}
}

Expand Down

0 comments on commit a34581d

Please sign in to comment.