Skip to content

Commit

Permalink
fix(expo): Reduce waning messages spam when a property in configurati…
Browse files Browse the repository at this point in the history
…on is missing (#3631)
  • Loading branch information
krystofwoldrich committed Feb 26, 2024
1 parent 9148964 commit 664fb8a
Show file tree
Hide file tree
Showing 7 changed files with 75 additions and 40 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
- You should not set the auth token in the plugin config except for local testing. Instead, use the `SENTRY_AUTH_TOKEN` env variable, as pointed out in our [docs](https://docs.sentry.io/platforms/react-native/manual-setup/expo/).
- 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/).
- Reduce waning messages spam when a property in Expo plugin configuration is missing ([#3631](https://github.com/getsentry/sentry-react-native/pull/3631))

## 5.19.0

Expand Down
31 changes: 30 additions & 1 deletion plugin/src/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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<string, boolean>();
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 };
30 changes: 15 additions & 15 deletions plugin/src/withSentry.ts
Original file line number Diff line number Diff line change
@@ -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';

Expand All @@ -25,18 +25,12 @@ const withSentryPlugin: ConfigPlugin<PluginProps | void> = (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}`);
}
}

Expand All @@ -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}
Expand Down
7 changes: 3 additions & 4 deletions plugin/src/withSentryAndroid.ts
Original file line number Diff line number Diff line change
@@ -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<string> = (config, sentryProperties: string) => {
const cfg = withAppBuildGradle(config, config => {
Expand Down Expand Up @@ -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;
Expand Down
30 changes: 20 additions & 10 deletions plugin/src/withSentryIOS.ts
Original file line number Diff line number Diff line change
@@ -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 };

Expand Down Expand Up @@ -46,14 +46,24 @@ export const withSentryIOS: ConfigPlugin<string> = (config, sentryProperties: st
};

export function modifyExistingXcodeBuildScript(script: BuildPhase): void {
if (
!script.shellScript.match(/(packager|scripts)\/react-native-xcode\.sh\b/) ||
script.shellScript.includes('sentry-xcode.sh') ||
script.shellScript.includes('@sentry')
) {
WarningAggregator.addWarningIOS(
SDK_PACKAGE_NAME,
"Unable to modify build script 'Bundle React Native code and images'. Please open a bug report at https://github.com/expo/sentry-expo.",
if (!script.shellScript.match(/(packager|scripts)\/react-native-xcode\.sh\b/)) {
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`,
);
return;
}

if (script.shellScript.includes('sentry-xcode.sh')) {
warnOnce("The latest 'sentry-xcode.sh' script already exists in 'Bundle React Native code and images'.");
return;
}

if (script.shellScript.includes('@sentry')) {
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`,
);
return;
}
Expand Down
8 changes: 3 additions & 5 deletions test/expo-plugin/modifyAppBuildGradle.test.ts
Original file line number Diff line number Diff line change
@@ -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")
Expand Down Expand Up @@ -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();
});
});
8 changes: 3 additions & 5 deletions test/expo-plugin/modifyXcodeProject.test.ts
Original file line number Diff line number Diff line change
@@ -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(`"
Expand Down Expand Up @@ -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();
});
});

0 comments on commit 664fb8a

Please sign in to comment.