Fix(replays): console.error not rendering properly (#35877)

Now renders the messsage and ignore data.arguments when there are no placeholders in message or objects to print. 

Closes #35701

static/app/components/events/meta/annotatedText/index.tsx

@@ -86,7 +86,7 @@ const AnnotatedText = ({value, meta, className, ...props}: Props) => {
   };
 
   return (
-    <span className={className} {...props}>
+    <span role="text" className={className} {...props}>
       {renderValue()}
       {meta?.err && renderErrors(meta.err)}
     </span>

static/app/types/breadcrumbs.tsx

@@ -10,6 +10,7 @@ export enum BreadcrumbLevelType {
   INFO = 'info',
   DEBUG = 'debug',
   UNDEFINED = 'undefined',
+  LOG = 'log',
 }
 
 export enum BreadcrumbType {

static/app/views/replays/detail/console/consoleMessage.tsx

@@ -1,5 +1,6 @@
 import {Fragment} from 'react';
 import styled from '@emotion/styled';
+import isObject from 'lodash/isObject';
 import {sprintf, vsprintf} from 'sprintf-js';
 
 import DateTime from 'sentry/components/dateTime';
@@ -12,6 +13,7 @@ import Tooltip from 'sentry/components/tooltip';
 import {IconClose, IconWarning} from 'sentry/icons';
 import space from 'sentry/styles/space';
 import {BreadcrumbTypeDefault} from 'sentry/types/breadcrumbs';
+import {objectIsEmpty} from 'sentry/utils';
 
 interface MessageFormatterProps {
   breadcrumb: BreadcrumbTypeDefault;
@@ -35,7 +37,9 @@ function renderString(arg: string | number | boolean | Object) {
 /**
  * Attempt to emulate the browser console as much as possible
  */
-function MessageFormatter({breadcrumb}: MessageFormatterProps) {
+export function MessageFormatter({breadcrumb}: MessageFormatterProps) {
+  let logMessage = '';
+
   // Browser's console formatter only works on the first arg
   const [message, ...args] = breadcrumb.data?.arguments;
 
@@ -45,35 +49,57 @@ function MessageFormatter({breadcrumb}: MessageFormatterProps) {
     ? sprintf.parse(message).filter(parsed => Array.isArray(parsed))
     : [];
 
-  // TODO `%c` is console specific, it applies colors to messages
-  // for now we are stripping it as this is potentially risky to implement due to xss
-  const consoleColorPlaceholderIndexes = placeholders
-    .filter(([placeholder]) => placeholder === '%c')
-    .map((_, i) => i);
-
-  // Retrieve message formatting args
-  const messageArgs = args.slice(0, placeholders.length);
-
-  // Filter out args that were for %c
-  for (const colorIndex of consoleColorPlaceholderIndexes) {
-    messageArgs.splice(colorIndex, 1);
+  // Placeholders can only occur in the first argument and only if it is a string.
+  // We can skip the below code and avoid using `sprintf` if there are no placeholders.
+  if (placeholders.length) {
+    // TODO `%c` is console specific, it applies colors to messages
+    // for now we are stripping it as this is potentially risky to implement due to xss
+    const consoleColorPlaceholderIndexes = placeholders
+      .filter(([placeholder]) => placeholder === '%c')
+      .map((_, i) => i);
+
+    // Retrieve message formatting args
+    const messageArgs = args.slice(0, placeholders.length);
+
+    // Filter out args that were for %c
+    for (const colorIndex of consoleColorPlaceholderIndexes) {
+      messageArgs.splice(colorIndex, 1);
+    }
+
+    // Attempt to stringify the rest of the args
+    const restArgs = args.slice(placeholders.length).map(renderString);
+
+    const formattedMessage = isMessageString
+      ? vsprintf(message.replaceAll('%c', ''), messageArgs)
+      : renderString(message);
+
+    logMessage = [formattedMessage, ...restArgs].join(' ').trim();
+  } else if (
+    breadcrumb.data?.arguments.length === 1 &&
+    isObject(message) &&
+    objectIsEmpty(message)
+  ) {
+    // There is a special case where `console.error()` is called with an Error object.
+    // The SDK uses the Error's `message` property as the breadcrumb message, but we lose the Error type,
+    // resulting in an empty object in the breadcrumb arguments. In this case, we
+    // only want to use `breadcrumb.message`.
+    logMessage = breadcrumb.message || JSON.stringify(message);
+  } else {
+    // If the string `[object Object]` is found in message, it means the SDK attempted to stringify an object,
+    // but the actual object should be captured in the arguments.
+    //
+    // Likewise if arrays are found e.g. [test,test] the SDK will serialize it to 'test, test'.
+    //
+    // In those cases, we'll want to use our pretty print in every argument that was passed to the logger instead of using
+    // the SDK's serialization.
+    const argValues = breadcrumb.data?.arguments.map(renderString);
+
+    logMessage = argValues.join(' ').trim();
   }
 
-  // Attempt to stringify the rest of the args
-  const restArgs = args.slice(placeholders.length).map(renderString);
-
-  const formattedMessage = isMessageString
-    ? vsprintf(message.replaceAll('%c', ''), messageArgs)
-    : renderString(message);
-
   // TODO(replays): Add better support for AnnotatedText (e.g. we use message
   // args from breadcrumb.data.arguments and not breadcrumb.message directly)
-  return (
-    <AnnotatedText
-      meta={getMeta(breadcrumb, 'message')}
-      value={[formattedMessage, ...restArgs].join(' ')}
-    />
-  );
+  return <AnnotatedText meta={getMeta(breadcrumb, 'message')} value={logMessage} />;
 }
 
 interface ConsoleMessageProps extends MessageFormatterProps {

tests/js/spec/views/replays/detail/console/messageFormatter.spec.tsx

@@ -0,0 +1,120 @@
+import {render, screen} from 'sentry-test/reactTestingLibrary';
+
+import {
+  BreadcrumbLevelType,
+  BreadcrumbType,
+  BreadcrumbTypeDefault,
+} from 'sentry/types/breadcrumbs';
+import {MessageFormatter} from 'sentry/views/replays/detail/console/consoleMessage';
+
+const breadcrumbs: BreadcrumbTypeDefault[] = [
+  {
+    type: BreadcrumbType.DEBUG,
+    category: 'console',
+    data: {
+      arguments: ['This is a %s', 'test'],
+      logger: 'console',
+    },
+    level: BreadcrumbLevelType.LOG,
+    message: 'This is a %s test',
+    timestamp: '2022-06-22T20:00:39.959Z',
+  },
+  {
+    type: BreadcrumbType.DEBUG,
+    category: 'console',
+    data: {
+      arguments: ['test', 1, false, {}],
+      logger: 'console',
+    },
+    level: BreadcrumbLevelType.LOG,
+    message: 'test 1 false [object Object]',
+    timestamp: '2022-06-22T16:49:11.198Z',
+  },
+  {
+    type: BreadcrumbType.DEBUG,
+    category: 'console',
+    data: {
+      arguments: [{}],
+      logger: 'console',
+    },
+    level: BreadcrumbLevelType.ERROR,
+    message: 'Error: this is my error message',
+    timestamp: '2022-06-22T20:00:39.958Z',
+  },
+  {
+    type: BreadcrumbType.DEBUG,
+    category: 'console',
+    data: {
+      arguments: [{}],
+      logger: 'console',
+    },
+    level: BreadcrumbLevelType.ERROR,
+    timestamp: '2022-06-22T20:00:39.958Z',
+  },
+  {
+    type: BreadcrumbType.DEBUG,
+    category: 'console',
+    data: {
+      arguments: [
+        '%c prev state',
+        'color: #9E9E9E; font-weight: bold',
+        {
+          cart: [],
+        },
+      ],
+      logger: 'console',
+    },
+    level: BreadcrumbLevelType.LOG,
+    message: '%c prev state color: #9E9E9E; font-weight: bold [object Object]',
+    timestamp: '2022-06-09T00:50:25.273Z',
+  },
+  {
+    type: BreadcrumbType.DEBUG,
+    category: 'console',
+    data: {
+      arguments: ['test', ['foo', 'bar']],
+      logger: 'console',
+    },
+    level: BreadcrumbLevelType.LOG,
+    message: 'test foo,bar',
+    timestamp: '2022-06-23T17:09:31.158Z',
+  },
+];
+
+describe('MessageFormatter', () => {
+  it('Should print console message with placeholders correctly', () => {
+    render(<MessageFormatter breadcrumb={breadcrumbs[0]} />);
+
+    expect(screen.getByRole('text')).toHaveTextContent('This is a test');
+  });
+
+  it('Should print console message with objects correctly', () => {
+    render(<MessageFormatter breadcrumb={breadcrumbs[1]} />);
+
+    expect(screen.getByRole('text')).toHaveTextContent('test 1 false {}');
+  });
+
+  it('Should print console message correctly when it is an Error object', () => {
+    render(<MessageFormatter breadcrumb={breadcrumbs[2]} />);
+
+    expect(screen.getByRole('text')).toHaveTextContent('Error: this is my error message');
+  });
+
+  it('Should print empty object in case there is no message prop', () => {
+    render(<MessageFormatter breadcrumb={breadcrumbs[3]} />);
+
+    expect(screen.getByRole('text')).toHaveTextContent('{}');
+  });
+
+  it('Should ignore the "%c" placheholder and print the console message correctly', () => {
+    render(<MessageFormatter breadcrumb={breadcrumbs[4]} />);
+
+    expect(screen.getByRole('text')).toHaveTextContent('prev state {"cart":[]}');
+  });
+
+  it('Should print arrays correctly', () => {
+    render(<MessageFormatter breadcrumb={breadcrumbs[5]} />);
+
+    expect(screen.getByRole('text')).toHaveTextContent('test ["foo","bar"]');
+  });
+});