diff --git a/eslint-suppressions.json b/eslint-suppressions.json index c818b780f6..6fa740978b 100644 --- a/eslint-suppressions.json +++ b/eslint-suppressions.json @@ -1058,11 +1058,6 @@ "count": 1 } }, - "src/logger/transports/sentry.ts": { - "@typescript-eslint/no-unsafe-enum-comparison": { - "count": 3 - } - }, "src/logger/types.ts": { "@typescript-eslint/no-redundant-type-constituents": { "count": 1 diff --git a/src/App.tsx b/src/App.tsx index d41f28bdb5..03481ce012 100644 --- a/src/App.tsx +++ b/src/App.tsx @@ -124,7 +124,7 @@ function InnerApp() { await features.init } } catch (e) { - logger.error(`session: resume failed`, {message: e}) + logger.warn(`session: resume failed`, {message: e}) } finally { setIsReady(true) } diff --git a/src/App.web.tsx b/src/App.web.tsx index e962728859..aa8550bbc2 100644 --- a/src/App.web.tsx +++ b/src/App.web.tsx @@ -103,7 +103,7 @@ function InnerApp() { await features.init } } catch (e) { - logger.error('session: resumeSession failed', {message: e}) + logger.warn('session: resumeSession failed', {message: e}) } finally { setIsReady(true) } diff --git a/src/components/ContextMenu/index.tsx b/src/components/ContextMenu/index.tsx index 4a6f453a8a..b1eb873216 100644 --- a/src/components/ContextMenu/index.tsx +++ b/src/components/ContextMenu/index.tsx @@ -132,7 +132,7 @@ export function Root({children}: {children: React.ReactNode}) { const onHoverableTouchUp = useCallback((id: string) => { const hoverable = hoverables.current.get(id) if (!hoverable) { - logger.warn(`No such hoverable with id ${id}`) + logger.warn(`No such hoverable`, {id}) return } hoverable.onTouchUp() diff --git a/src/components/Post/Embed/VideoEmbed/VideoEmbedInner/web-controls/utils.tsx b/src/components/Post/Embed/VideoEmbed/VideoEmbedInner/web-controls/utils.tsx index 38b48fbe2c..164b214ce7 100644 --- a/src/components/Post/Embed/VideoEmbed/VideoEmbedInner/web-controls/utils.tsx +++ b/src/components/Post/Embed/VideoEmbed/VideoEmbedInner/web-controls/utils.tsx @@ -189,7 +189,7 @@ export function useVideoElement(ref: RefObject) { `The play() request was interrupted by a call to pause()`, ) ) { - logger.error('Error playing video:', {message: err}) + logger.warn('Error playing video:', {message: err}) } }) } diff --git a/src/lib/notifications/notifications.ts b/src/lib/notifications/notifications.ts index 008b2ef73f..6351a8d8e1 100644 --- a/src/lib/notifications/notifications.ts +++ b/src/lib/notifications/notifications.ts @@ -55,7 +55,7 @@ async function _registerPushToken({ notyLogger.debug(`registerPushToken: success`) } catch (error) { if (!isNetworkError(error)) { - notyLogger.error(`registerPushToken: failed`, {safeMessage: error}) + notyLogger.warn(`registerPushToken: failed`, {safeMessage: error}) } } } diff --git a/src/lib/translation/index.tsx b/src/lib/translation/index.tsx index 0e7b182a62..1f1f8d4e58 100644 --- a/src/lib/translation/index.tsx +++ b/src/lib/translation/index.tsx @@ -287,7 +287,7 @@ export function Provider({children}: React.PropsWithChildren) { })) } catch (err) { const e = err as Error - logger.error('Failed to translate text on device', {safeMessage: e}) + logger.warn('Failed to translate text on device', {safeMessage: e}) // On-device translation failed (language pack missing or user // dismissed the download prompt). ax.metric('translate:result', { diff --git a/src/logger/__tests__/logger.test.ts b/src/logger/__tests__/logger.test.ts index 2f6915a4b3..d7bda5ad23 100644 --- a/src/logger/__tests__/logger.test.ts +++ b/src/logger/__tests__/logger.test.ts @@ -182,11 +182,7 @@ describe('general functionality', () => { timestamp: sentryTimestamp, }) jest.runAllTimers() - expect(Sentry.captureMessage).toHaveBeenCalledWith(message, { - level: 'log', - tags: {category: 'logger'}, - extra: {__context__: 'logger'}, - }) + expect(Sentry.captureMessage).not.toHaveBeenCalled() sentryTransport( LogLevel.Warn, @@ -204,8 +200,18 @@ describe('general functionality', () => { timestamp: sentryTimestamp, }) jest.runAllTimers() + expect(Sentry.captureMessage).not.toHaveBeenCalled() + + sentryTransport( + LogLevel.Error, + Logger.Context.Default, + message, + {}, + timestamp, + ) + jest.runAllTimers() expect(Sentry.captureMessage).toHaveBeenCalledWith(message, { - level: 'warning', + level: 'error', tags: {category: 'logger'}, extra: {__context__: 'logger'}, }) @@ -259,6 +265,42 @@ describe('general functionality', () => { }) }) + test('sentryTransport filters network errors', () => { + jest.clearAllMocks() + const timestamp = Date.now() + + // network error in the message itself + sentryTransport( + LogLevel.Error, + Logger.Context.Default, + 'Network request failed', + {}, + timestamp, + ) + + // network error in metadata, message is something else + sentryTransport( + LogLevel.Error, + Logger.Context.Default, + 'poll failed', + {safeMessage: new Error('Network request failed')}, + timestamp, + ) + + jest.runAllTimers() + expect(Sentry.captureMessage).not.toHaveBeenCalled() + + // network Error object + sentryTransport( + LogLevel.Error, + Logger.Context.Default, + new Error('Network request failed'), + {}, + timestamp, + ) + expect(Sentry.captureException).not.toHaveBeenCalled() + }) + test('add/remove transport', () => { const timestamp = Date.now() const logger = new Logger({}) diff --git a/src/logger/transports/sentry.ts b/src/logger/transports/sentry.ts index 5453279015..1764e91541 100644 --- a/src/logger/transports/sentry.ts +++ b/src/logger/transports/sentry.ts @@ -47,16 +47,24 @@ export const sentryTransport: Transport = ( timestamp: timestamp / 1000, // Sentry expects seconds }) - // We don't want to send any network errors to sentry - if (isNetworkError(message)) { + // We don't want to send any network errors to sentry. The underlying + // cause is often passed via metadata rather than the message itself, so + // check the common metadata keys too. + if ( + isNetworkError(message) || + isNetworkError(metadata.safeMessage) || + isNetworkError(metadata.message) || + isNetworkError(metadata.error) + ) { return } /** - * Send all higher levels with `captureMessage`, with appropriate severity - * level + * Only error-level strings are reported to Sentry as events. Lower levels + * are captured as breadcrumbs above and attached to the next event, if + * any. */ - if (level === 'error' || level === 'warn' || level === 'log') { + if (level === LogLevel.Error) { // Defer non-critical messages so they're sent in a batch queueMessageForSentry(message, { level: severity, @@ -65,6 +73,11 @@ export const sentryTransport: Transport = ( }) } } else { + // We don't want to send any network errors to sentry + if (isNetworkError(message)) { + return + } + /** * It's otherwise an Error and should be reported with captureException */ diff --git a/src/screens/Login/ChooseAccountForm.tsx b/src/screens/Login/ChooseAccountForm.tsx index 041e2e3b22..8ce8cf4bc2 100644 --- a/src/screens/Login/ChooseAccountForm.tsx +++ b/src/screens/Login/ChooseAccountForm.tsx @@ -55,7 +55,7 @@ export const ChooseAccountForm = ({ }) Toast.show(_(msg`Signed in as @${account.handle}`)) } catch (e: any) { - logger.error('choose account: initSession failed', { + logger.warn('choose account: initSession failed', { message: e instanceof Error ? e.message : 'Unknown error', }) // Move to login form. diff --git a/src/state/messages/events/agent.ts b/src/state/messages/events/agent.ts index 7794204447..56e6bed173 100644 --- a/src/state/messages/events/agent.ts +++ b/src/state/messages/events/agent.ts @@ -392,7 +392,7 @@ export class MessagesEventBus { } } catch (e: any) { if (!isNetworkError(e) && !isErrorMaybeAppPasswordPermissions(e)) { - logger.error(`poll events failed`, { + logger.warn(`poll events failed`, { safeMessage: e.message, }) } diff --git a/src/view/com/posts/PostFeed.tsx b/src/view/com/posts/PostFeed.tsx index 20c6a31a3a..e502ca028c 100644 --- a/src/view/com/posts/PostFeed.tsx +++ b/src/view/com/posts/PostFeed.tsx @@ -308,7 +308,7 @@ let PostFeed = ({ } } catch (e) { if (!isNetworkError(e)) { - logger.error('Poll latest failed', {feed, message: String(e)}) + logger.warn('Poll latest failed', {feed, message: String(e)}) } } })