feat: Use new logger and ReanimatedError for lib warnings and errors #6387

MatiPl01 · 2024-08-07T14:47:18Z

Summary

This PR replaces all console.warn, console.error and throw new Error statements with custom logger calls (logger.warn, logger.error and logger.newError respectively).

It declares the logger object as a global variable for easier usage and to prevent circular imports (as it imports runOnJS, which was the cause of circular dependencies).

Test plan

open the EmptyExample in the example app,
change the SHOW_EXAMPLE constant to see the respective error/warning,
see better error logs with code frame that was the culprit (if possible - sometimes we cannot get pretty error stack trace, especially when functions are called asynchronously)

MatiPl01 · 2024-08-07T16:02:21Z

Logs comparison

(Based on 17 examples that I tested out)

Improved logs

1. Error: [Reanimated] Property `text` was whitelisted both as UI and native prop. Please remove it from one of the lists.

Before	After

2. Warning: Error: [Reanimated] Function `getViewProp` requires a component to be passed as an argument on Fabric.

Before	After

3. Warning: Error: [Reanimated] Invalid color space provided: INVALID. Supported values are: ['RGB', 'HSV'].

Before	After

4. Warning: Error: [Reanimated] Unsupported value for "interpolate"

Before	After

5. (A bit different) Warning: Error: [Reanimated] Unsupported value for "interpolate"

Before	After

6. Warning: Error: [Reanimated] Interpolation input and output ranges should contain at least two values.

Before	After

7. Warning: Error: [Reanimated] The function passed to `runOnRuntime` is not a worklet.

Before	After

8. [Reanimated] No animation was provided for the sequence

Before	After

9. Warning: Error: [Reanimated] The easing function is not a worklet. Please make sure you import `Easing` from react-native-reanimated.

Before	After

10. Warning: Error: [Reanimated] `useAnimatedStyle` has to return an object, found undefined instead.

Before	After

11. [Reanimated] The view with tag -1 is not a valid argument for measure(). This may be because the view is not currently rendered, which may not be a bug (e.g. an off-screen FlatList item).

Before	After

12. [Reanimated] setNativeProps() can only be used on the UI runtime.

Before	After

Not prettified logs

These logs were pointing to the log call location in reanimated (without user's code in the stack trace showing the real cause of the issue). Because of that I decided to remove code fragment from these logs as it was a bit misleading.

1. [Reanimated] Property "opacity" of AnimatedComponent(View) may be overwritten by a layout animation. Please wrap your component with an animated view and apply the layout animation on the wrapper.

Before	After

2. ReanimatedError: [Reanimated] Bezier x values must be in [0, 1] range., js engine: reanimated

Before	After

3. Warning: Error: [Reanimated] Reading from value directly is not supported. Use `worklet` to read value from the animation state.

Before	After

4. [Reanimated] Wrong config was provided to withClamp. Min value is bigger than max

Before	After

5. [Reanimated] Invalid spring config, stiffness must be grater than zero but got 0, damping must be grater than zero but got 0

Before	After

apps/common-app/src/examples/EmptyExample.tsx

packages/react-native-reanimated/src/Bezier.ts

apps/next-example/babel.config.js

packages/react-native-reanimated/jest-setup.js

packages/react-native-reanimated/jest.config.js

packages/react-native-reanimated/src/Bezier.ts

packages/react-native-reanimated/src/layoutReanimation/web/domUtils.ts

packages/react-native-reanimated/src/logger/logger.ts

packages/react-native-reanimated/src/mutables.ts

packages/react-native-reanimated/src/publicGlobals.ts

packages/react-native-reanimated/src/runtimes.ts

packages/react-native-reanimated/plugin/src/class.ts

packages/react-native-reanimated/src/ConfigHelper.ts

packages/react-native-reanimated/src/PropsRegistry.ts

tjzel

All looks good!

Since now we want to enforce the use of ReanimatedError and logger in our source code, please add a CI step that ensures no plain errors (except places where we must) or no console.something is used.

I guess we'd need to make an ESLint plugin for that, so don't consider it a priority task.

packages/react-native-reanimated/jest.config.js

packages/react-native-reanimated/plugin/src/globals.ts

packages/react-native-reanimated/src/index.ts

packages/react-native-reanimated/src/screenTransition/RNScreensTurboModule.ts

## Summary This PR is a much cleaner approach than proposed in #6364. It includes metro-config modification which is essential to collapse logs from reanimated source code, which aren't helpful to the user while tracking down the issue. The previous approach was trimming logs from reanimated source code completely - this approach just collapses them, so that they are still available to the user and can be revealed above the presented stack trace part. ## General idea To get better logs, I had to implement the following 2 changes: 1. **metro config** - the `symbolicator` must have been added to properly collapse stack frames that aren't meaningful to the user, 2. **logger object** - the new logger object uses `LogBox.addLog` method, thanks to which we can get pretty stack traces when we log a warning from the UI thread (before such warnings didn't include meaningful stack trace as error stack was created inside `LogBox` after `runOnJS` was called, so we were getting a bit limited JS stack - see [example 11](#6387 (comment)) in the follow up PR). ## Example improvement (tested on a real project to see if it works there as well) - current logs either point to the reanimated source code or aren't readable at all (if warning is logged from the UI thread as in the example below) - new logger shows correct destination of the issue culprit in the code frame, collapses stack frames in the call stack that aren't interesting to the user (reanimated source code) and focuses on the file where the user included problematic code | Before | After | |-|-| | <video src="https://github.com/user-attachments/assets/a5302586-f4d0-4636-8bd8-6c406c9d8c73" /> | <video src="https://github.com/user-attachments/assets/3121636f-69a2-4b6f-8f38-b1889d4c62e1" /> | ## Test plan See the example in the next PR (#6387). --------- Co-authored-by: Tomasz Żelawski <[email protected]>

Co-authored-by: Tomasz Żelawski <[email protected]>

## Summary This PR is a much cleaner approach than proposed in #6364. It includes metro-config modification which is essential to collapse logs from reanimated source code, which aren't helpful to the user while tracking down the issue. The previous approach was trimming logs from reanimated source code completely - this approach just collapses them, so that they are still available to the user and can be revealed above the presented stack trace part. ## General idea To get better logs, I had to implement the following 2 changes: 1. **metro config** - the `symbolicator` must have been added to properly collapse stack frames that aren't meaningful to the user, 2. **logger object** - the new logger object uses `LogBox.addLog` method, thanks to which we can get pretty stack traces when we log a warning from the UI thread (before such warnings didn't include meaningful stack trace as error stack was created inside `LogBox` after `runOnJS` was called, so we were getting a bit limited JS stack - see [example 11](#6387 (comment)) in the follow up PR). ## Example improvement (tested on a real project to see if it works there as well) - current logs either point to the reanimated source code or aren't readable at all (if warning is logged from the UI thread as in the example below) - new logger shows correct destination of the issue culprit in the code frame, collapses stack frames in the call stack that aren't interesting to the user (reanimated source code) and focuses on the file where the user included problematic code | Before | After | |-|-| | <video src="https://github.com/user-attachments/assets/a5302586-f4d0-4636-8bd8-6c406c9d8c73" /> | <video src="https://github.com/user-attachments/assets/3121636f-69a2-4b6f-8f38-b1889d4c62e1" /> | ## Test plan See the example in the next PR (#6387). --------- Co-authored-by: Tomasz Żelawski <[email protected]>

This PR is a much cleaner approach than proposed in #6364. It includes metro-config modification which is essential to collapse logs from reanimated source code, which aren't helpful to the user while tracking down the issue. The previous approach was trimming logs from reanimated source code completely - this approach just collapses them, so that they are still available to the user and can be revealed above the presented stack trace part. To get better logs, I had to implement the following 2 changes: 1. **metro config** - the `symbolicator` must have been added to properly collapse stack frames that aren't meaningful to the user, 2. **logger object** - the new logger object uses `LogBox.addLog` method, thanks to which we can get pretty stack traces when we log a warning from the UI thread (before such warnings didn't include meaningful stack trace as error stack was created inside `LogBox` after `runOnJS` was called, so we were getting a bit limited JS stack - see [example 11](#6387 (comment)) in the follow up PR). (tested on a real project to see if it works there as well) - current logs either point to the reanimated source code or aren't readable at all (if warning is logged from the UI thread as in the example below) - new logger shows correct destination of the issue culprit in the code frame, collapses stack frames in the call stack that aren't interesting to the user (reanimated source code) and focuses on the file where the user included problematic code | Before | After | |-|-| | <video src="https://github.com/user-attachments/assets/a5302586-f4d0-4636-8bd8-6c406c9d8c73" /> | <video src="https://github.com/user-attachments/assets/3121636f-69a2-4b6f-8f38-b1889d4c62e1" /> | See the example in the next PR (#6387). --------- Co-authored-by: Tomasz Żelawski <[email protected]>

…6387) ## Summary This PR replaces all `console.warn`, `console.error` and `throw new Error` statements with custom logger calls (`logger.warn`, `logger.error` and `logger.newError` respectively). It declares the logger object as a global variable for easier usage and to prevent circular imports (as it imports `runOnJS`, which was the cause of circular dependencies). ## Test plan - open the `EmptyExample` in the example app, - change the `SHOW_EXAMPLE` constant to see the respective error/warning, - see better error logs with code frame that was the culprit (if possible - sometimes we cannot get pretty error stack trace, especially when functions are called asynchronously) --------- Co-authored-by: Tomasz Żelawski <[email protected]>

MatiPl01 self-assigned this Aug 7, 2024

MatiPl01 mentioned this pull request Aug 7, 2024

feat: Implement better reanimated logger with clean stack traces #6385

Merged

MatiPl01 commented Aug 7, 2024

View reviewed changes

apps/common-app/src/examples/EmptyExample.tsx Outdated Show resolved Hide resolved

MatiPl01 changed the title ~~feat: Replace console with custom logger where possible~~ feat: Replace console with custom logger Aug 7, 2024

MatiPl01 commented Aug 8, 2024

View reviewed changes

packages/react-native-reanimated/src/Bezier.ts Outdated Show resolved Hide resolved

MatiPl01 marked this pull request as ready for review August 8, 2024 09:37

MatiPl01 requested review from tomekzaw and tjzel August 8, 2024 09:48

tjzel requested changes Aug 8, 2024

View reviewed changes

MatiPl01 force-pushed the @matipl01/new-logger-console-logs-replacement branch from 775c87a to 3d68cac Compare August 9, 2024 10:51

MatiPl01 changed the title ~~feat: Replace console with custom logger~~ feat: Use new logger and ReanimatedError for lib warnings and errors Aug 9, 2024

MatiPl01 force-pushed the @matipl01/new-logger-console-logs-replacement branch 2 times, most recently from f7ab541 to 573192c Compare August 12, 2024 17:40

MatiPl01 requested a review from tjzel August 12, 2024 17:56

MatiPl01 force-pushed the @matipl01/new-logger-console-logs-replacement branch from c187063 to 7b1a40c Compare August 12, 2024 18:08

tjzel reviewed Aug 16, 2024

View reviewed changes

packages/react-native-reanimated/plugin/src/class.ts Outdated Show resolved Hide resolved

packages/react-native-reanimated/src/ConfigHelper.ts Show resolved Hide resolved

packages/react-native-reanimated/src/PropsRegistry.ts Outdated Show resolved Hide resolved

MatiPl01 force-pushed the @matipl01/new-logger-console-logs-replacement branch from 386b83a to fba8f74 Compare August 23, 2024 15:15

MatiPl01 requested a review from tjzel August 23, 2024 15:21

tjzel approved these changes Aug 26, 2024

View reviewed changes

Base automatically changed from @matipl01/better-logs to main August 26, 2024 10:17

MatiPl01 added 8 commits August 26, 2024 12:24

Remove ReanimatedError from globals and replace with import

bc95741

Start new logger refactor

b0c257d

Continue changing console to logger

2b594e2

Remove formatting changes from cpp files

175093f

Fix linting issue

a61fbce

Declare logger as a global variable to fix circular imports

70304df

Another attempt to fix CI failures

218df70

Revert cpp file changes, add missing 'use strict'

a9977cf

MatiPl01 and others added 10 commits August 26, 2024 12:24

Remove no longer needed globals declaration in jest config

8d83e30

Refactor logger according to suggestions

1561803

Fix ReanimatedError issues

3eb0909

Update packages/react-native-reanimated/src/hook/utils.ts

4ecefa6

Co-authored-by: Tomasz Żelawski <[email protected]>

Update packages/react-native-reanimated/src/runtimes.ts

09496a2

Co-authored-by: Tomasz Żelawski <[email protected]>

Fix CI errors

b9e65bd

Update packages/react-native-reanimated/src/PropsRegistry.ts

b7d2f31

Co-authored-by: Tomasz Żelawski <[email protected]>

Ensure ReanimatedError is added to globals before it is used

e5860ec

Add requested changes, revert EmptyExample

dd2c6de

Fis issues after rebase

8986ccb

MatiPl01 force-pushed the @matipl01/new-logger-console-logs-replacement branch from d5d9145 to 8986ccb Compare August 26, 2024 10:37

Remove generated plugin build files

ceb7193

MatiPl01 enabled auto-merge August 26, 2024 10:45

MatiPl01 added this pull request to the merge queue Aug 26, 2024

Merged via the queue into main with commit afbad1e Aug 26, 2024
8 checks passed

MatiPl01 deleted the @matipl01/new-logger-console-logs-replacement branch August 26, 2024 12:02

This was referenced Nov 8, 2024

[Snyk] Upgrade react-native-reanimated from 3.10.1 to 3.16.0 inrupt/inrupt-data-wallet#94

Open

[Snyk] Upgrade react-native-reanimated from 3.10.1 to 3.16.1 inrupt/inrupt-data-wallet#95

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Use new logger and ReanimatedError for lib warnings and errors #6387

feat: Use new logger and ReanimatedError for lib warnings and errors #6387

MatiPl01 commented Aug 7, 2024 •

edited

Loading

MatiPl01 commented Aug 7, 2024 •

edited

Loading

tjzel left a comment

feat: Use new logger and ReanimatedError for lib warnings and errors #6387

feat: Use new logger and ReanimatedError for lib warnings and errors #6387

Conversation

MatiPl01 commented Aug 7, 2024 • edited Loading

Summary

Test plan

MatiPl01 commented Aug 7, 2024 • edited Loading

Logs comparison

Improved logs

1. Error: [Reanimated] Property text was whitelisted both as UI and native prop. Please remove it from one of the lists.

2. Warning: Error: [Reanimated] Function getViewProp requires a component to be passed as an argument on Fabric.

3. Warning: Error: [Reanimated] Invalid color space provided: INVALID. Supported values are: ['RGB', 'HSV'].

4. Warning: Error: [Reanimated] Unsupported value for "interpolate"

5. (A bit different) Warning: Error: [Reanimated] Unsupported value for "interpolate"

6. Warning: Error: [Reanimated] Interpolation input and output ranges should contain at least two values.

7. Warning: Error: [Reanimated] The function passed to runOnRuntime is not a worklet.

8. [Reanimated] No animation was provided for the sequence

9. Warning: Error: [Reanimated] The easing function is not a worklet. Please make sure you import Easing from react-native-reanimated.

10. Warning: Error: [Reanimated] useAnimatedStyle has to return an object, found undefined instead.

11. [Reanimated] The view with tag -1 is not a valid argument for measure(). This may be because the view is not currently rendered, which may not be a bug (e.g. an off-screen FlatList item).

12. [Reanimated] setNativeProps() can only be used on the UI runtime.

Not prettified logs

1. [Reanimated] Property "opacity" of AnimatedComponent(View) may be overwritten by a layout animation. Please wrap your component with an animated view and apply the layout animation on the wrapper.

2. ReanimatedError: [Reanimated] Bezier x values must be in [0, 1] range., js engine: reanimated

3. Warning: Error: [Reanimated] Reading from value directly is not supported. Use worklet to read value from the animation state.

4. [Reanimated] Wrong config was provided to withClamp. Min value is bigger than max

5. [Reanimated] Invalid spring config, stiffness must be grater than zero but got 0, damping must be grater than zero but got 0

tjzel left a comment

Choose a reason for hiding this comment

MatiPl01 commented Aug 7, 2024 •

edited

Loading

MatiPl01 commented Aug 7, 2024 •

edited

Loading

1. Error: [Reanimated] Property `text` was whitelisted both as UI and native prop. Please remove it from one of the lists.

2. Warning: Error: [Reanimated] Function `getViewProp` requires a component to be passed as an argument on Fabric.

7. Warning: Error: [Reanimated] The function passed to `runOnRuntime` is not a worklet.

9. Warning: Error: [Reanimated] The easing function is not a worklet. Please make sure you import `Easing` from react-native-reanimated.

10. Warning: Error: [Reanimated] `useAnimatedStyle` has to return an object, found undefined instead.

3. Warning: Error: [Reanimated] Reading from value directly is not supported. Use `worklet` to read value from the animation state.