From db2a30f3b3b8db7417e3047a740f0b8a3eb6977f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Kry=C5=A1tof=20Wold=C5=99ich?= <31292499+krystofwoldrich@users.noreply.github.com> Date: Mon, 26 Feb 2024 15:51:21 +0100 Subject: [PATCH] fix(expo): Reduce waning messages spam when a property in configuration is missing (#3631) --- CHANGELOG.md | 1 + plugin/src/utils.ts | 31 ++++++++++++++++++- plugin/src/withSentry.ts | 30 +++++++++--------- plugin/src/withSentryAndroid.ts | 7 ++--- plugin/src/withSentryIOS.ts | 15 +++------ test/expo-plugin/modifyAppBuildGradle.test.ts | 8 ++--- test/expo-plugin/modifyXcodeProject.test.ts | 8 ++--- 7 files changed, 60 insertions(+), 40 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 28baa38dd..82fcc4318 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,7 @@ - In addition to showing a warning, we are now actively removing an `authToken` from the plugin config if it was set. - If you had set the auth token in the plugin config previously, **and** built and published an app with that config, you should [rotate your token](https://docs.sentry.io/product/accounts/auth-tokens/). - Ignore JSON response when retrieving source context from local Expo Dev Server ([#3611](https://github.com/getsentry/sentry-react-native/pull/3611)) +- Reduce waning messages spam when a property in Expo plugin configuration is missing ([#3631](https://github.com/getsentry/sentry-react-native/pull/3631)) ### Dependencies diff --git a/plugin/src/utils.ts b/plugin/src/utils.ts index 22320d247..c587426b4 100644 --- a/plugin/src/utils.ts +++ b/plugin/src/utils.ts @@ -15,6 +15,35 @@ const sdkPackage: { // eslint-disable-next-line @typescript-eslint/no-var-requires } = require('../../package.json'); -const SDK_PACKAGE_NAME = sdkPackage.name; +const SDK_PACKAGE_NAME = `${sdkPackage.name}/expo`; + +const warningMap = new Map(); +export function warnOnce(message: string): void { + if (!warningMap.has(message)) { + warningMap.set(message, true); + // eslint-disable-next-line no-console + console.warn(yellow(`${logPrefix()} ${message}`)); + } +} + +export function logPrefix(): string { + return `› ${bold('[@sentry/react-native/expo]')}`; +} + +/** + * The same as `chalk.yellow` + * This code is part of the SDK, we don't want to introduce a dependency on `chalk` just for this. + */ +export function yellow(message: string): string { + return `\x1b[33m${message}\x1b[0m`; +} + +/** + * The same as `chalk.bold` + * This code is part of the SDK, we don't want to introduce a dependency on `chalk` just for this. + */ +export function bold(message: string): string { + return `\x1b[1m${message}\x1b[22m`; +} export { sdkPackage, SDK_PACKAGE_NAME }; diff --git a/plugin/src/withSentry.ts b/plugin/src/withSentry.ts index acf6ff4d6..0ad0db94b 100644 --- a/plugin/src/withSentry.ts +++ b/plugin/src/withSentry.ts @@ -1,7 +1,7 @@ import type { ConfigPlugin } from 'expo/config-plugins'; -import { createRunOncePlugin, WarningAggregator } from 'expo/config-plugins'; +import { createRunOncePlugin } from 'expo/config-plugins'; -import { SDK_PACKAGE_NAME, sdkPackage } from './utils'; +import { bold, sdkPackage, warnOnce } from './utils'; import { withSentryAndroid } from './withSentryAndroid'; import { withSentryIOS } from './withSentryIOS'; @@ -25,18 +25,12 @@ const withSentryPlugin: ConfigPlugin = (config, props) => { try { cfg = withSentryAndroid(cfg, sentryProperties); } catch (e) { - WarningAggregator.addWarningAndroid( - SDK_PACKAGE_NAME, - `There was a problem configuring sentry-expo in your native Android project: ${e}`, - ); + warnOnce(`There was a problem with configuring your native Android project: ${e}`); } try { cfg = withSentryIOS(cfg, sentryProperties); } catch (e) { - WarningAggregator.addWarningIOS( - SDK_PACKAGE_NAME, - `There was a problem configuring sentry-expo in your native iOS project: ${e}`, - ); + warnOnce(`There was a problem with configuring your native iOS project: ${e}`); } } @@ -54,11 +48,17 @@ export function getSentryProperties(props: PluginProps | void): string | null { const missingProperties = ['organization', 'project'].filter(each => !props?.hasOwnProperty(each)); if (missingProperties.length) { - const warningMessage = `Missing Sentry configuration properties: ${missingProperties.join( - ', ', - )} in config plugin. Builds will fall back to environment variables. See: https://docs.sentry.io/platforms/react-native/manual-setup/.`; - WarningAggregator.addWarningAndroid(SDK_PACKAGE_NAME, warningMessage); - WarningAggregator.addWarningIOS(SDK_PACKAGE_NAME, warningMessage); + const missingPropertiesString = bold(missingProperties.join(', ')); + const warningMessage = `Missing config for ${missingPropertiesString}. Environment variables will be used as a fallback during the build. https://docs.sentry.io/platforms/react-native/manual-setup/`; + warnOnce(warningMessage); + } + + if (authToken) { + warnOnce( + `Detected unsecure use of 'authToken' in Sentry plugin configuration. To avoid exposing the token use ${bold( + 'SENTRY_AUTH_TOKEN', + )} environment variable instead. https://docs.sentry.io/platforms/react-native/manual-setup/`, + ); } return `defaults.url=${url} diff --git a/plugin/src/withSentryAndroid.ts b/plugin/src/withSentryAndroid.ts index 74e187be4..9beaa2388 100644 --- a/plugin/src/withSentryAndroid.ts +++ b/plugin/src/withSentryAndroid.ts @@ -1,8 +1,8 @@ import type { ConfigPlugin } from 'expo/config-plugins'; -import { WarningAggregator, withAppBuildGradle, withDangerousMod } from 'expo/config-plugins'; +import { withAppBuildGradle, withDangerousMod } from 'expo/config-plugins'; import * as path from 'path'; -import { SDK_PACKAGE_NAME, writeSentryPropertiesTo } from './utils'; +import { warnOnce, writeSentryPropertiesTo } from './utils'; export const withSentryAndroid: ConfigPlugin = (config, sentryProperties: string) => { const cfg = withAppBuildGradle(config, config => { @@ -39,8 +39,7 @@ export function modifyAppBuildGradle(buildGradle: string): string { const pattern = /^android {/m; if (!buildGradle.match(pattern)) { - WarningAggregator.addWarningAndroid( - SDK_PACKAGE_NAME, + warnOnce( 'Could not find `^android {` in `android/app/build.gradle`. Please open a bug report at https://github.com/getsentry/sentry-react-native.', ); return buildGradle; diff --git a/plugin/src/withSentryIOS.ts b/plugin/src/withSentryIOS.ts index c33cd8d6b..db2526183 100644 --- a/plugin/src/withSentryIOS.ts +++ b/plugin/src/withSentryIOS.ts @@ -1,9 +1,9 @@ /* eslint-disable @typescript-eslint/no-unsafe-member-access */ import type { ConfigPlugin, XcodeProject } from 'expo/config-plugins'; -import { WarningAggregator, withDangerousMod, withXcodeProject } from 'expo/config-plugins'; +import { withDangerousMod, withXcodeProject } from 'expo/config-plugins'; import * as path from 'path'; -import { SDK_PACKAGE_NAME, writeSentryPropertiesTo } from './utils'; +import { warnOnce, writeSentryPropertiesTo } from './utils'; type BuildPhase = { shellScript: string }; @@ -47,8 +47,7 @@ export const withSentryIOS: ConfigPlugin = (config, sentryProperties: st export function modifyExistingXcodeBuildScript(script: BuildPhase): void { if (!script.shellScript.match(/(packager|scripts)\/react-native-xcode\.sh\b/)) { - WarningAggregator.addWarningIOS( - SDK_PACKAGE_NAME, + warnOnce( `'react-native-xcode.sh' not found in 'Bundle React Native code and images'. Please open a bug report at https://github.com/getsentry/sentry-react-native`, ); @@ -56,16 +55,12 @@ Please open a bug report at https://github.com/getsentry/sentry-react-native`, } if (script.shellScript.includes('sentry-xcode.sh')) { - WarningAggregator.addWarningIOS( - SDK_PACKAGE_NAME, - "The latest 'sentry-xcode.sh' script already exists in 'Bundle React Native code and images'.", - ); + warnOnce("The latest 'sentry-xcode.sh' script already exists in 'Bundle React Native code and images'."); return; } if (script.shellScript.includes('@sentry')) { - WarningAggregator.addWarningIOS( - SDK_PACKAGE_NAME, + warnOnce( `Outdated or custom Sentry script found in 'Bundle React Native code and images'. Regenerate the native project to use the latest script. Run npx expo prebuild --clean`, diff --git a/test/expo-plugin/modifyAppBuildGradle.test.ts b/test/expo-plugin/modifyAppBuildGradle.test.ts index 0a0933524..e1363983d 100644 --- a/test/expo-plugin/modifyAppBuildGradle.test.ts +++ b/test/expo-plugin/modifyAppBuildGradle.test.ts @@ -1,8 +1,7 @@ -import { WarningAggregator } from 'expo/config-plugins'; - +import { warnOnce } from '../../plugin/src/utils'; import { modifyAppBuildGradle } from '../../plugin/src/withSentryAndroid'; -jest.mock('expo/config-plugins'); +jest.mock('../../plugin/src/utils'); const buildGradleWithSentry = ` apply from: new File(["node", "--print", "require('path').dirname(require.resolve('@sentry/react-native/package.json'))"].execute().text.trim(), "sentry.gradle") @@ -49,8 +48,7 @@ describe('Configures Android native project correctly', () => { }); it('Warns to file a bug report if no react.gradle is found', () => { - (WarningAggregator.addWarningAndroid as jest.Mock).mockImplementationOnce(jest.fn()); modifyAppBuildGradle(buildGradleWithOutReactGradleScript); - expect(WarningAggregator.addWarningAndroid).toHaveBeenCalled(); + expect(warnOnce).toHaveBeenCalled(); }); }); diff --git a/test/expo-plugin/modifyXcodeProject.test.ts b/test/expo-plugin/modifyXcodeProject.test.ts index 1bdf6bbd1..bbd570fdf 100644 --- a/test/expo-plugin/modifyXcodeProject.test.ts +++ b/test/expo-plugin/modifyXcodeProject.test.ts @@ -1,8 +1,7 @@ -import { WarningAggregator } from 'expo/config-plugins'; - +import { warnOnce } from '../../plugin/src/utils'; import { modifyExistingXcodeBuildScript } from '../../plugin/src/withSentryIOS'; -jest.mock('expo/config-plugins'); +jest.mock('../../plugin/src/utils'); const buildScriptWithoutSentry = { shellScript: JSON.stringify(`" @@ -74,8 +73,7 @@ describe('Configures iOS native project correctly', () => { }); it("Warns to file a bug report if build script isn't what we expect to find", () => { - (WarningAggregator.addWarningAndroid as jest.Mock).mockImplementationOnce(jest.fn()); modifyExistingXcodeBuildScript(buildScriptWeDontExpect); - expect(WarningAggregator.addWarningIOS).toHaveBeenCalled(); + expect(warnOnce).toHaveBeenCalled(); }); });