Steve-Dee-Designs
/
woocommerce
mirror of https://github.com/woocommerce/woocommerce.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Integrate JS remote logging package in WooCommerce Admin (#50134) 

* Update remote logger to check dev env and whether logging is enabled

* Add changelog

* Integrate JS remote logging package in WooCommerce Admin

* Add changelog

* Update remote logger package

* Update test

* Log error in error boundary

* Update remote logger

* Fix webpack config

* Update log stack format

* Update handleError

* Add init debug
This commit is contained in:
Chi-Hsuan Huang 2024-08-02 10:25:39 +08:00 committed by GitHub
parent a3f4d722b9
commit ed81aa8201
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
16 changed files with 280 additions and 47 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

5
packages/js/remote-logging/README.md
View File

@ -28,7 +28,7 @@ The WooCommerce Remote Logging package offers the following features:
2. Log messages or errors:
```js
import { log } from '@woocommerce/remote-logging';
import { log, captureException } from '@woocommerce/remote-logging';
// Log an informational message
log('info', 'User completed checkout', { extra: { orderId: '12345' } });
@ -40,7 +40,7 @@ The WooCommerce Remote Logging package offers the following features:
try {
// Some operation that might throw
} catch (error) {
log('error', 'Failed to process order', { extra: { error } });
captureException(error, { extra: { orderId: '12345' } });
}
```
@ -76,5 +76,6 @@ addFilter(
- `init(config: RemoteLoggerConfig): void`: Initializes the remote logger with the given configuration.
- `log(severity: LogSeverity, message: string, extraData?: object): Promise<void>`: Logs a message with the specified severity and optional extra data.
- `captureException(error: Error, extraData?: object): void`: Captures an error and sends it to the remote API.
For more detailed information about types and interfaces, refer to the source code and inline documentation.

4
packages/js/remote-logging/changelog/add-wcadmin-remote-logging Normal file
View File

@ -0,0 +1,4 @@
Significance: patch
Type: update
Update remote logger to check dev env and whether logging is enabled

1
packages/js/remote-logging/src/index.ts
View File

@ -1,6 +1,7 @@
export {
init,
log,
captureException,
REMOTE_LOGGING_SHOULD_SEND_ERROR_FILTER,
REMOTE_LOGGING_ERROR_DATA_FILTER,
REMOTE_LOGGING_LOG_ENDPOINT_FILTER,

142
packages/js/remote-logging/src/remote-logger.ts
View File

@ -9,7 +9,7 @@ import { applyFilters } from '@wordpress/hooks';
/**
* Internal dependencies
*/
import { mergeLogData } from './utils';
import { mergeLogData, isDevelopmentEnvironment } from './utils';
import { LogData, ErrorData, RemoteLoggerConfig } from './types';
const debug = debugFactory( 'wc:remote-logging' );
@ -80,9 +80,38 @@ export class RemoteLogger {
...extraData,
} );
debug( 'Logging:', logData );
await this.sendLog( logData );
}
/**
* Logs an error to Logstash.
*
* @param error - The error to log.
* @param extraData - Optional additional data to include in the log.
*
* @return {Promise<void>} - A promise that resolves when the error is logged.
*/
public async error( error: Error, extraData?: Partial< LogData > ) {
if ( this.isRateLimited() ) {
return;
}
const errorData: ErrorData = {
...mergeLogData( DEFAULT_LOG_DATA, {
message: error.message,
severity: 'error',
...extraData,
} ),
trace: this.getFormattedStackFrame(
TraceKit.computeStackTrace( error )
),
};
debug( 'Logging error:', errorData );
await this.sendError( errorData );
}
/**
* Initializes error event listeners for catching unhandled errors and unhandled rejections.
*/
@ -115,6 +144,11 @@ export class RemoteLogger {
* @param logData - The log data to be sent.
*/
private async sendLog( logData: LogData ): Promise< void > {
if ( isDevelopmentEnvironment ) {
debug( 'Skipping send log in development environment' );
return;
}
const body = new window.FormData();
body.append( 'params', JSON.stringify( logData ) );
@ -142,24 +176,18 @@ export class RemoteLogger {
}
/**
* Handles an error and prepares it for sending to the remote API.
* Handles an uncaught error and sends it to the remote API.
*
* @param error - The error to handle.
*/
private async handleError( error: Error ) {
const currentTime = Date.now();
if (
currentTime - this.lastErrorSentTime <
this.config.errorRateLimitMs
) {
debug( 'Rate limit reached. Skipping send error', error );
const trace = TraceKit.computeStackTrace( error );
if ( ! this.shouldHandleError( error, trace.stack ) ) {
debug( 'Irrelevant error. Skipping handling.', error );
return;
}
const trace = TraceKit.computeStackTrace( error );
if ( ! this.shouldSendError( error, trace.stack ) ) {
debug( 'Skipping error:', error );
if ( this.isRateLimited() ) {
return;
}
@ -196,12 +224,15 @@ export class RemoteLogger {
* @param error - The error data to be sent.
*/
private async sendError( error: ErrorData ) {
if ( isDevelopmentEnvironment ) {
debug( 'Skipping send error in development environment' );
return;
}
const body = new window.FormData();
body.append( 'error', JSON.stringify( error ) );
try {
debug( 'Sending error to API:', error );
/**
* Filters the JS error endpoint URL.
*
@ -212,6 +243,8 @@ export class RemoteLogger {
'https://public-api.wordpress.com/rest/v1.1/js-error'
) as string;
debug( 'Sending error to API:', error );
await window.fetch( endpoint, {
method: 'POST',
body,
@ -279,13 +312,13 @@ export class RemoteLogger {
}
/**
* Determines whether an error should be sent to the remote API.
* Determines whether an error should be handled.
*
* @param error - The error to check.
* @param stackFrames - The stack frames of the error.
* @return Whether the error should be sent.
* @return Whether the error should be handled.
*/
private shouldSendError(
private shouldHandleError(
error: Error,
stackFrames: TraceKit.StackFrame[]
) {
@ -310,10 +343,41 @@ export class RemoteLogger {
stackFrames
) as boolean;
}
private isRateLimited(): boolean {
const currentTime = Date.now();
if (
currentTime - this.lastErrorSentTime <
this.config.errorRateLimitMs
) {
debug( 'Rate limit reached. Skipping send error' );
return true;
}
return false;
}
}
let logger: RemoteLogger | null = null;
/**
* Checks if remote logging is enabled and if the logger is initialized.
*
* @return {boolean} - Returns true if remote logging is enabled and the logger is initialized, otherwise false.
*/
function canLog(): boolean {
if ( ! getSetting( 'isRemoteLoggingEnabled', false ) ) {
debug( 'Remote logging is disabled.' );
return false;
}
if ( ! logger ) {
warnLog( 'RemoteLogger is not initialized. Call init() first.' );
return false;
}
return true;
}
/**
* Initializes the remote logging and error handlers.
* This function should be called once at the start of the application.
@ -321,9 +385,9 @@ let logger: RemoteLogger | null = null;
* @param config - Configuration object for the RemoteLogger.
*
*/
export function init( config: RemoteLoggerConfig ): void {
if ( ! window.wcTracks || ! window.wcTracks.isEnabled ) {
debug( 'Tracks is not enabled.' );
export function init( config: RemoteLoggerConfig ) {
if ( ! getSetting( 'isRemoteLoggingEnabled', false ) ) {
debug( 'Remote logging is disabled.' );
return;
}
@ -335,38 +399,56 @@ export function init( config: RemoteLoggerConfig ): void {
try {
logger = new RemoteLogger( config );
logger.initializeErrorHandlers();
debug( 'RemoteLogger initialized.' );
} catch ( error ) {
errorLog( 'Failed to initialize RemoteLogger:', error );
}
}
/**
* Logs a message or error, respecting rate limiting.
* Logs a message or error.
*
* This function is inefficient because the data goes over the REST API, so use sparingly.
*
* @param severity - The severity of the log.
* @param message - The message to log.
* @param extraData - Optional additional data to include in the log.
*
*/
export async function log(
severity: Exclude< LogData[ 'severity' ], undefined >,
message: string,
extraData?: Partial< Exclude< LogData, 'message' | 'severity' > >
) {
if ( ! window.wcTracks || ! window.wcTracks.isEnabled ) {
debug( 'Tracks is not enabled.' );
return;
}
if ( ! logger ) {
warnLog( 'RemoteLogger is not initialized. Call init() first.' );
): Promise< void > {
if ( ! canLog() ) {
return;
}
try {
await logger.log( severity, message, extraData );
await logger?.log( severity, message, extraData );
} catch ( error ) {
errorLog( 'Failed to send log:', error );
}
}
/**
* Captures an error and sends it to the remote API. Respects the error rate limit.
*
* @param error - The error to capture.
* @param extraData - Optional additional data to include in the log.
*/
export async function captureException(
error: Error,
extraData?: Partial< LogData >
) {
if ( ! canLog() ) {
return;
}
try {
await logger?.error( error, extraData );
} catch ( _error ) {
errorLog( 'Failed to send log:', _error );
}
}

122
packages/js/remote-logging/src/test/index.ts
View File

@ -3,10 +3,12 @@
*/
import '@wordpress/jest-console';
import { addFilter, removeFilter } from '@wordpress/hooks';
import { getSetting } from '@woocommerce/settings';
/**
* Internal dependencies
*/
import { init, log } from '../';
import { init, log, captureException } from '../';
import {
RemoteLogger,
REMOTE_LOGGING_SHOULD_SEND_ERROR_FILTER,
@ -16,6 +18,12 @@ import {
} from '../remote-logger';
import { fetchMock } from './__mocks__/fetch';
jest.mock( '@woocommerce/settings', () => ( {
getSetting: jest.fn().mockReturnValue( {
isRemoteLoggingEnabled: true,
} ),
} ) );
jest.mock( 'tracekit', () => ( {
computeStackTrace: jest.fn().mockReturnValue( {
name: 'Error',
@ -93,6 +101,51 @@ describe( 'RemoteLogger', () => {
} );
} );
describe( 'error', () => {
it( 'should send an error to the API with default data', async () => {
const error = new Error( 'Test error' );
await logger.error( error );
expect( fetchMock ).toHaveBeenCalledWith(
'https://public-api.wordpress.com/rest/v1.1/js-error',
expect.objectContaining( {
method: 'POST',
body: expect.any( FormData ),
} )
);
const formData = fetchMock.mock.calls[0][1].body;
const payload = JSON.parse(formData.get('error'));
expect( payload['message'] ).toBe( 'Test error' );
expect( payload['severity'] ).toBe( 'error' );
expect( payload['trace'] ).toContain( '#1 at testFunction (http://example.com/woocommerce/assets/js/admin/app.min.js:1:1)' );
} );
it( 'should send an error to the API with extra data', async () => {
const error = new Error( 'Test error' );
const extraData = {
severity: 'warning' as const,
tags: ['custom-tag'],
};
await logger.error( error, extraData );
expect( fetchMock ).toHaveBeenCalledWith(
'https://public-api.wordpress.com/rest/v1.1/js-error',
expect.objectContaining( {
method: 'POST',
body: expect.any( FormData ),
} )
);
const formData = fetchMock.mock.calls[0][1].body;
const payload = JSON.parse(formData.get('error'));
expect( payload['message'] ).toBe( 'Test error' );
expect( payload['severity'] ).toBe( 'warning' );
expect( payload['tags'] ).toEqual( ["woocommerce", "js", "custom-tag"]);
expect( payload['trace'] ).toContain( '#1 at testFunction (http://example.com/woocommerce/assets/js/admin/app.min.js:1:1)' );
} );
} );
describe( 'handleError', () => {
it( 'should send an error to the API', async () => {
const error = new Error( 'Test error' );
@ -164,7 +217,7 @@ describe( 'RemoteLogger', () => {
} );
} );
describe( 'shouldSendError', () => {
describe( 'shouldHandleError', () => {
it( 'should return true for WooCommerce errors', () => {
const error = new Error( 'Test error' );
const stackFrames = [
@ -176,7 +229,7 @@ describe( 'RemoteLogger', () => {
column: 1,
},
];
const result = ( logger as any ).shouldSendError(
const result = ( logger as any ).shouldHandleError(
error,
stackFrames
);
@ -194,7 +247,7 @@ describe( 'RemoteLogger', () => {
column: 1,
},
];
const result = ( logger as any ).shouldSendError(
const result = ( logger as any ).shouldHandleError(
error,
stackFrames
);
@ -203,7 +256,7 @@ describe( 'RemoteLogger', () => {
it( 'should return false for WooCommerce errors with no stack frames', () => {
const error = new Error( 'Test error' );
const result = ( logger as any ).shouldSendError( error, [] );
const result = ( logger as any ).shouldHandleError( error, [] );
expect( result ).toBe( false );
} );
@ -214,7 +267,7 @@ describe( 'RemoteLogger', () => {
() => true
);
const error = new Error( 'Test error' );
const result = ( logger as any ).shouldSendError( error, [] );
const result = ( logger as any ).shouldHandleError( error, [] );
expect( result ).toBe( true );
} );
} );
@ -255,18 +308,37 @@ describe( 'RemoteLogger', () => {
describe( 'init', () => {
beforeEach( () => {
jest.clearAllMocks();
window.wcTracks = { isEnabled: true };
( getSetting as jest.Mock ).mockImplementation(
( key, defaultValue ) => {
if ( key === 'isRemoteLoggingEnabled' ) {
return true;
}
return defaultValue;
}
);
} );
it( 'should not initialize the logger if Tracks is not enabled', () => {
window.wcTracks = { isEnabled: false };
it( 'should not initialize or log when remote logging is disabled', () => {
// Mock the getSetting function to return false for isRemoteLoggingEnabled
( getSetting as jest.Mock ).mockImplementation(
( key, defaultValue ) => {
if ( key === 'isRemoteLoggingEnabled' ) {
return false;
}
return defaultValue;
}
);
init( { errorRateLimitMs: 1000 } );
expect( () => log( 'info', 'Test message' ) ).not.toThrow();
log( 'info', 'Test message' );
expect( fetchMock ).not.toHaveBeenCalled();
} );
it( 'should initialize the logger if Tracks is enabled', () => {
it( 'should initialize and log without throwing when remote logging is enabled', () => {
init( { errorRateLimitMs: 1000 } );
expect( () => log( 'info', 'Test message' ) ).not.toThrow();
expect( fetchMock ).toHaveBeenCalled();
} );
it( 'should not initialize the logger twice', () => {
@ -280,9 +352,33 @@ describe( 'init', () => {
} );
describe( 'log', () => {
it( 'should not log if Tracks is not enabled', () => {
window.wcTracks = { isEnabled: false };
it( 'should not log if remote logging is disabled', () => {
( getSetting as jest.Mock ).mockImplementation(
( key, defaultValue ) => {
if ( key === 'isRemoteLoggingEnabled' ) {
return false;
}
return defaultValue;
}
);
log( 'info', 'Test message' );
expect( fetchMock ).not.toHaveBeenCalled();
} );
} );
describe( 'captureException', () => {
it( 'should not log error if remote logging is disabled', () => {
( getSetting as jest.Mock ).mockImplementation(
( key, defaultValue ) => {
if ( key === 'isRemoteLoggingEnabled' ) {
return false;
}
return defaultValue;
}
);
captureException( new Error( 'Test error' ) );
expect( fetchMock ).not.toHaveBeenCalled();
} );
} );

2
packages/js/remote-logging/src/utils.ts
View File

@ -39,3 +39,5 @@ export function mergeLogData( target: LogData, source: Partial< LogData > ) {
}
return result;
}
export const isDevelopmentEnvironment = process.env.NODE_ENV === 'development';

2
plugins/woocommerce-admin/client/core-profiler/index.tsx
View File

@ -43,6 +43,7 @@ import CurrencyFactory from '@woocommerce/currency';
* Internal dependencies
*/
import { findComponentMeta } from '~/utils/xstate/find-component';
import { initRemoteLogging } from '~/lib/init-remote-logging';
import { IntroOptIn } from './pages/IntroOptIn';
import {
UserProfile,
@ -330,6 +331,7 @@ const updateTrackingOption = fromPromise(
) {
window.wcTracks.enable( () => {
initializeExPlat();
initRemoteLogging();
resolve(); // resolve the promise only after explat is enabled by the callback
} );
} else {

18
plugins/woocommerce-admin/client/error-boundary/index.tsx
View File

@ -4,6 +4,7 @@
import { Component, ReactNode, ErrorInfo } from 'react';
import { __ } from '@wordpress/i18n';
import { Button } from '@wordpress/components';
import { captureException } from '@woocommerce/remote-logging';
/**
* Internal dependencies
*/
@ -34,9 +35,22 @@ export class ErrorBoundary extends Component<
return { hasError: true, error };
}
componentDidCatch( _error: Error, errorInfo: ErrorInfo ) {
componentDidCatch( error: Error, errorInfo: ErrorInfo ) {
this.setState( { errorInfo } );
// TODO: Log error to error tracking service
// Limit the component stack to 10 calls so we don't send too much data.
const componentStack = errorInfo.componentStack
.trim()
.split( '\n' )
.slice( 0, 10 )
.map( ( line ) => line.trim() );
captureException( error, {
severity: 'critical',
extra: {
componentStack,
},
} );
}
handleRefresh = () => {

6
plugins/woocommerce-admin/client/index.js
View File

@ -8,6 +8,12 @@ import {
withCurrentUserHydration,
withSettingsHydration,
} from '@woocommerce/data';
/**
* Internal dependencies
*/
import { initRemoteLogging } from './lib/init-remote-logging';
// Initialize remote logging early to log any errors that occur during initialization.
initRemoteLogging();
/**
* Internal dependencies

10
plugins/woocommerce-admin/client/lib/init-remote-logging.ts Normal file
View File

@ -0,0 +1,10 @@
/**
* External dependencies
*/
import { init } from '@woocommerce/remote-logging';
export const initRemoteLogging = () => {
init( {
errorRateLimitMs: 60000, // 1 minute
} );
};

4
plugins/woocommerce-admin/package.json
View File

@ -163,6 +163,7 @@
"@woocommerce/number": "workspace:*",
"@woocommerce/onboarding": "workspace:*",
"@woocommerce/product-editor": "workspace:*",
"@woocommerce/remote-logging": "workspace:*",
"@woocommerce/tracks": "workspace:*",
"@wordpress/babel-preset-default": "^6.17.0",
"@wordpress/block-editor": "^9.8.0",
@ -305,6 +306,9 @@
"node_modules/@woocommerce/tracks/build",
"node_modules/@woocommerce/tracks/build-module",
"node_modules/@woocommerce/tracks/build-types",
"node_modules/@woocommerce/remote-logging/build",
"node_modules/@woocommerce/remote-logging/build-module",
"node_modules/@woocommerce/remote-logging/build-types",
"node_modules/@woocommerce/product-editor/build",
"node_modules/@woocommerce/product-editor/build-module",
"node_modules/@woocommerce/product-editor/build-style",

1
plugins/woocommerce-admin/webpack.config.js
View File

@ -42,6 +42,7 @@ const wcAdminPackages = [
'onboarding',
'block-templates',
'product-editor',
'remote-logging',
];
// wpAdminScripts are loaded on wp-admin pages outside the context of WooCommerce Admin
// See ./client/wp-admin-scripts/README.md for more details

4
plugins/woocommerce/changelog/add-wcadmin-remote-logging Normal file
View File

@ -0,0 +1,4 @@
Significance: minor
Type: add
Integrate JS remote logging package in WooCommerce Admin

2
plugins/woocommerce/src/Blocks/Assets/AssetDataRegistry.php
View File

@ -3,6 +3,7 @@ namespace Automattic\WooCommerce\Blocks\Assets;
use Automattic\WooCommerce\Blocks\Package;
use Automattic\WooCommerce\Blocks\Domain\Services\Hydration;
use Automattic\WooCommerce\Internal\Logging\RemoteLogger;
use Exception;
use InvalidArgumentException;
@ -89,6 +90,7 @@ class AssetDataRegistry {
'dateFormat' => wc_date_format(),
'homeUrl' => esc_url( home_url( '/' ) ),
'locale' => $this->get_locale_data(),
'isRemoteLoggingEnabled' => wc_get_container()->get( RemoteLogger::class )->is_remote_logging_allowed(),
'dashboardUrl' => wc_get_account_endpoint_url( 'dashboard' ),
'orderStatuses' => $this->get_order_statuses(),
'placeholderImgSrc' => wc_placeholder_img_src(),

1
plugins/woocommerce/src/Internal/Admin/WCAdminAssets.php
View File

@ -279,6 +279,7 @@ class WCAdminAssets {
'wc-navigation',
'wc-block-templates',
'wc-product-editor',
'wc-remote-logging',
);
$scripts_map = array(

3
pnpm-lock.yaml
View File

@ -3600,6 +3600,9 @@ importers:
'@woocommerce/product-editor':
specifier: workspace:*
version: link:../../packages/js/product-editor
'@woocommerce/remote-logging':
specifier: workspace:*
version: link:../../packages/js/remote-logging
'@woocommerce/tracks':
specifier: workspace:*
version: link:../../packages/js/tracks