Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 28 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from 14 commits
Commits
Show all changes
28 commits
Select commit Hold shift + click to select a range
74e0a45
Another attempt to implement much better logger
MatiPl01 Aug 7, 2024
14eb336
Add custom logger
MatiPl01 Aug 7, 2024
49e70f1
Remove examples which will be added after logging refactor in the nex…
MatiPl01 Aug 7, 2024
f109d72
Remove formatting changes from cpp files
MatiPl01 Aug 7, 2024
374f794
Add missing console logs
MatiPl01 Aug 7, 2024
517539f
further improvements
MatiPl01 Aug 9, 2024
cefc844
Merge branch 'main' into @matipl01/better-logs
MatiPl01 Aug 9, 2024
bc75fcb
Fix failing next example
MatiPl01 Aug 9, 2024
dbbc18b
Apply review suggestions
MatiPl01 Aug 12, 2024
7768039
Merge branch 'main' into @matipl01/better-logs
MatiPl01 Aug 12, 2024
def4d34
Type fixes
MatiPl01 Aug 12, 2024
583e09d
Add proper implementation of ReanimatedError
MatiPl01 Aug 12, 2024
5aa710d
Fix web issues
MatiPl01 Aug 12, 2024
b581db5
Fix circular dependency
MatiPl01 Aug 12, 2024
b09fd16
Rename ReanimatedErrorFactory to ReanimatedErrorConstructor
MatiPl01 Aug 13, 2024
9cb4433
Update reanimated error according to review suggestions
MatiPl01 Aug 13, 2024
8a92f97
Merge changes from main
MatiPl01 Aug 23, 2024
446e24a
Update packages/react-native-reanimated/src/globals.d.ts
MatiPl01 Aug 23, 2024
5a1248f
Apply review suggestions, add ReanimatedError to custom worklet runtimes
MatiPl01 Aug 23, 2024
1228895
Remove global definition of ReanimatedError
MatiPl01 Aug 23, 2024
56edc88
Update packages/react-native-reanimated/src/threads.ts
MatiPl01 Aug 26, 2024
9f25693
Update packages/react-native-reanimated/src/threads.ts
MatiPl01 Aug 26, 2024
1bdde29
Update packages/react-native-reanimated/src/threads.ts
MatiPl01 Aug 26, 2024
76f2d90
Update packages/react-native-reanimated/src/errors.ts
MatiPl01 Aug 26, 2024
20d5437
Apply suggested changes
MatiPl01 Aug 26, 2024
a285348
Correct error message
MatiPl01 Aug 26, 2024
a96d53b
Revert changes in Podfile.lock
MatiPl01 Aug 26, 2024
55655e5
Fix worklet check in registerReanimatedError
MatiPl01 Aug 26, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion apps/fabric-example/metro.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');
const {
wrapWithReanimatedMetroConfig,
} = require('react-native-reanimated/metro-config');

const path = require('path');

Expand All @@ -8,4 +11,6 @@ const config = {
watchFolders: [root],
};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);
module.exports = wrapWithReanimatedMetroConfig(
mergeConfig(getDefaultConfig(__dirname), config)
);
7 changes: 6 additions & 1 deletion apps/macos-example/metro.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');
const {
wrapWithReanimatedMetroConfig,
} = require('react-native-reanimated/metro-config');

const path = require('path');
const exclusionList = require('metro-config/src/defaults/exclusionList');
Expand Down Expand Up @@ -30,4 +33,6 @@ const config = {
},
};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);
module.exports = wrapWithReanimatedMetroConfig(
mergeConfig(getDefaultConfig(__dirname), config)
);
7 changes: 6 additions & 1 deletion apps/paper-example/metro.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');
const {
wrapWithReanimatedMetroConfig,
} = require('react-native-reanimated/metro-config');

const path = require('path');

Expand All @@ -8,4 +11,6 @@ const config = {
watchFolders: [root],
};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);
module.exports = wrapWithReanimatedMetroConfig(
mergeConfig(getDefaultConfig(__dirname), config)
);
7 changes: 6 additions & 1 deletion apps/tvos-example/metro.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { getDefaultConfig, mergeConfig } = require('@react-native/metro-config');
const {
wrapWithReanimatedMetroConfig,
} = require('react-native-reanimated/metro-config');

const path = require('path');
const exclusionList = require('metro-config/src/defaults/exclusionList');
Expand Down Expand Up @@ -29,4 +32,6 @@ const config = {
},
};

module.exports = mergeConfig(getDefaultConfig(__dirname), config);
module.exports = wrapWithReanimatedMetroConfig(
mergeConfig(getDefaultConfig(__dirname), config)
);
5 changes: 4 additions & 1 deletion apps/web-example/metro.config.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,7 @@
const { getDefaultConfig } = require('expo/metro-config');
const {
wrapWithReanimatedMetroConfig,
} = require('react-native-reanimated/metro-config');

const path = require('path');
const exclusionList = require('metro-config/src/defaults/exclusionList');
Expand Down Expand Up @@ -27,4 +30,4 @@ config.resolver.blacklistRE = exclusionList(
)
);

module.exports = config;
module.exports = wrapWithReanimatedMetroConfig(config);
3 changes: 3 additions & 0 deletions packages/react-native-reanimated/metro-config/index.d.ts
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
import type { ConfigT } from 'metro-config';

export declare function wrapWithReanimatedMetroConfig(config: ConfigT): ConfigT;
35 changes: 35 additions & 0 deletions packages/react-native-reanimated/metro-config/index.js
tjzel marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
const COLLAPSED_STACK_REGEX = new RegExp(
[
// For internal usage in the example app
'/packages/react-native-reanimated/.+\\.(t|j)sx?$',
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
// When reanimated is installed as a dependency (node_modules)
'/node_modules/react-native-reanimated/.+\\.(t|j)sx?$',
]
// Make patterns work with both Windows and POSIX paths.
.map((pathPattern) => pathPattern.replaceAll('/', '[/\\\\]'))
.join('|')
);

function wrapWithReanimatedMetroConfig(config) {
return {
...config,
symbolicator: {
async customizeFrame(frame) {
const collapse = Boolean(
// Collapse the stack frame based on user's config symbolicator settings
(await config?.symbolicator?.customizeFrame?.(frame))?.collapse ||
// or, if not already collapsed, collapse the stack frame with path
// to react-native-reanimated source code
(frame.file && COLLAPSED_STACK_REGEX.test(frame.file))
);
return {
collapse,
};
},
},
};
}

module.exports = {
wrapWithReanimatedMetroConfig,
};
1 change: 1 addition & 0 deletions packages/react-native-reanimated/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@
"scripts/reanimated_utils.rb",
"mock.js",
"plugin/index.js",
"metro-config",
"plugin/build/plugin.js",
"!**/__tests__",
"!**/__fixtures__",
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native-reanimated/plugin/build/plugin.js

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

1 change: 1 addition & 0 deletions packages/react-native-reanimated/plugin/src/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,7 @@ const notCapturedIdentifiers = [

// Reanimated
'_WORKLET',
'ReanimatedError',
];

/**
Expand Down
17 changes: 17 additions & 0 deletions packages/react-native-reanimated/src/errors.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,23 @@
'use strict';
import type { WorkletStackDetails } from './commonTypes';

export interface ReanimatedError extends Error {
// eslint-disable-next-line @typescript-eslint/no-misused-new
new (message: string): ReanimatedError;
}

const ReanimatedErrorFactory: ReanimatedError = ((message: string) => {
'worklet';
const errorInstance = new Error(`[Reanimated] ${message}`);
errorInstance.name = 'ReanimatedError';
return errorInstance;
}) as unknown as ReanimatedError;
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved

export function registerReanimatedError() {
'worklet';
global.ReanimatedError = ReanimatedErrorFactory;
}

const _workletStackDetails = new Map<number, WorkletStackDetails>();

export function registerWorkletStackDetails(
Expand Down
2 changes: 2 additions & 0 deletions packages/react-native-reanimated/src/globals.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type { UpdatePropsManager } from './UpdateProps';
import type { callGuardDEV } from './initializers';
import type { WorkletRuntime } from './runtimes';
import type { RNScreensTurboModuleType } from './screenTransition/commonTypes';
import type { ReanimatedError } from './errors';

declare global {
var _REANIMATED_IS_REDUCED_MOTION: boolean | undefined;
Expand Down Expand Up @@ -112,4 +113,5 @@ declare global {
shadowNodeWrapper: ShadowNodeWrapper,
propName: string
) => string;
var ReanimatedError: ReanimatedError;
}
4 changes: 3 additions & 1 deletion packages/react-native-reanimated/src/initializers.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
'use strict';
import { reportFatalErrorOnJS } from './errors';
import { reportFatalErrorOnJS, registerReanimatedError } from './errors';
import { isChromeDebugger, isJest, shouldBeUseWeb } from './PlatformChecker';
import {
runOnJS,
Expand All @@ -9,6 +9,8 @@ import {
} from './threads';
import { mockedRequestAnimationFrame } from './mockedRequestAnimationFrame';

registerReanimatedError();
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved

const IS_JEST = isJest();
const SHOULD_BE_USE_WEB = shouldBeUseWeb();
const IS_CHROME_DEBUGGER = isChromeDebugger();
Expand Down
56 changes: 56 additions & 0 deletions packages/react-native-reanimated/src/logger/LogBox.ts
tjzel marked this conversation as resolved.
Show resolved Hide resolved
Original file line number Diff line number Diff line change
@@ -0,0 +1,56 @@
'use strict';
/**
* Copied from:
* react-native/Libraries/LogBox/Data/LogBoxData.js
* react-native/Libraries/LogBox/Data/parseLogBoxLog.js
*/

import type { LogBoxStatic } from 'react-native';
import { LogBox as RNLogBox } from 'react-native';

export type LogLevel = 'warn' | 'error' | 'fatal' | 'syntax';

type Message = {
content: string;
substitutions: { length: number; offset: number }[];
};

type Category = string;

interface Location {
row: number;
column: number;
}

interface CodeFrame {
content: string;
location?: Location | null;
fileName: string;
collapse?: boolean;
}

type ComponentStack = CodeFrame[];

type ComponentStackType = 'legacy' | 'stack';

export type LogData = {
level: LogLevel;
message: Message;
category: Category;
componentStack: ComponentStack;
componentStackType: ComponentStackType | null;
stack?: string;
};

interface LogBoxExtended extends LogBoxStatic {
addLog(data: LogData): void;
}
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved

const LogBox = RNLogBox as LogBoxExtended;

const noop = () => {
// do nothing
};

// Do nothing when addLogBoxLog is called if LogBox is not available
export const addLogBoxLog = LogBox?.addLog?.bind(LogBox) ?? noop;
3 changes: 3 additions & 0 deletions packages/react-native-reanimated/src/logger/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
'use strict';
export * from './logger';
export * from './LogBox';
69 changes: 69 additions & 0 deletions packages/react-native-reanimated/src/logger/logger.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
'use strict';
import { addLogBoxLog } from './LogBox';
import type { LogLevel, LogData } from './LogBox';

function logToConsole(data: LogData) {
'worklet';
switch (data.level) {
case 'warn':
console.warn(data.message.content);
break;
case 'error':
case 'fatal':
case 'syntax':
console.error(data.message.content);
break;
}
}

function formatMessage(message: string) {
'worklet';
return `[Reanimated] ${message}`;
}

function createLog(level: LogLevel, message: string): LogData {
'worklet';
const formattedMessage = formatMessage(message);

return {
level,
message: {
content: formattedMessage,
substitutions: [],
},
category: formattedMessage,
componentStack: [],
componentStackType: null,
stack: new Error().stack,
};
}

const loggerImpl = {
logFunction: logToConsole,
};

export function logToLogBoxAndConsole(data: LogData) {
addLogBoxLog(data);
logToConsole(data);
}

export function replaceLoggerImplementation(
logFunction: (data: LogData) => void
) {
loggerImpl.logFunction = logFunction;
}

export const logger = {
warn(message: string) {
'worklet';
loggerImpl.logFunction(createLog('warn', message));
},
error(message: string) {
'worklet';
loggerImpl.logFunction(createLog('error', message));
},
fatal(message: string) {
'worklet';
loggerImpl.logFunction(createLog('fatal', message));
},
};
36 changes: 29 additions & 7 deletions packages/react-native-reanimated/src/threads.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,14 @@ import {
makeShareableCloneRecursive,
} from './shareables';
import { isWorkletFunction } from './commonTypes';
import type { LogData } from './logger';
import {
logger,
logToLogBoxAndConsole,
replaceLoggerImplementation,
} from './logger';
import { registerReanimatedError } from './errors';
import { shareableMappingCache } from './shareableMappingCache';

const IS_JEST = isJest();
const SHOULD_BE_USE_WEB = shouldBeUseWeb();
Expand Down Expand Up @@ -76,12 +84,12 @@ export function runOnUI<Args extends unknown[], ReturnValue>(
): (...args: Args) => void {
'worklet';
if (__DEV__ && !SHOULD_BE_USE_WEB && _WORKLET) {
throw new Error(
'[Reanimated] `runOnUI` cannot be called on the UI runtime. Please call the function synchronously or use `queueMicrotask` or `requestAnimationFrame` instead.'
throw new ReanimatedError(
'`runOnUI` cannot be called on the UI runtime. Please call the function synchronously or use `queueMicrotask` or `requestAnimationFrame` instead.'
);
}
if (__DEV__ && !SHOULD_BE_USE_WEB && !isWorkletFunction(worklet)) {
throw new Error('[Reanimated] `runOnUI` can only be used on worklets.');
throw new ReanimatedError('`runOnUI` can only be used on worklets.');
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
}
return (...args) => {
if (IS_JEST) {
Expand Down Expand Up @@ -162,13 +170,13 @@ export function runOnUIImmediately<Args extends unknown[], ReturnValue>(
): (...args: Args) => void {
'worklet';
if (__DEV__ && !SHOULD_BE_USE_WEB && _WORKLET) {
throw new Error(
'[Reanimated] `runOnUIImmediately` cannot be called on the UI runtime. Please call the function synchronously or use `queueMicrotask` or `requestAnimationFrame` instead.'
throw new ReanimatedError(
'`runOnUIImmediately` cannot be called on the UI runtime. Please call the function synchronously or use `queueMicrotask` or `requestAnimationFrame` instead.'
);
}
if (__DEV__ && !SHOULD_BE_USE_WEB && !isWorkletFunction(worklet)) {
throw new Error(
'[Reanimated] `runOnUIImmediately` can only be used on worklets.'
throw new ReanimatedError(
'`runOnUIImmediately` can only be used on worklets.'
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
);
}
return (...args) => {
Expand Down Expand Up @@ -255,3 +263,17 @@ export function runOnJS<Args extends unknown[], ReturnValue>(
);
};
}

// Override the logFunction implementation with the one that adds logs
// with better stack traces to the LogBox (need to override it after runOnJS
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
// is defined).
replaceLoggerImplementation((data: LogData) => {
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
'worklet';
runOnJS(logToLogBoxAndConsole)(data);
});
shareableMappingCache.set(logger, makeShareableCloneRecursive(logger));

// Register ReanimatedError in the UI global scope
if (!shouldBeUseWeb()) {
MatiPl01 marked this conversation as resolved.
Show resolved Hide resolved
executeOnUIRuntimeSync(registerReanimatedError)();
}
Loading