Convert error reporting and tracing integrations to plugins by alexeyr-ci · Pull Request #471 · shakacode/react_on_rails_pro

alexeyr-ci · 2024-11-24T17:43:38Z

Reworks error reporting and tracing integrations to:

Allow the configuration script to provide any options to the services according to the user's requirements.
Allow integration of other services without modifying the renderer code.

Summary by CodeRabbit

Release Notes

New Features
- Introduced enhanced error reporting and tracing capabilities with integrations for Honeybadger and Sentry.
- Added support for streaming server rendering, optimizing performance for React components.
- New helper methods for capturing and replaying console logs during server rendering.
- Added new configuration options for managing error handling during server rendering.
- Enhanced ESLint configuration for better TypeScript support.
- New functions for error and message notification management in the API.
Bug Fixes
- Improved error handling during server rendering and within suspense boundaries.
Documentation
- Updated documentation on error reporting and tracing integration, including example usage.
Chores
- Updated dependency versions and configuration options for improved flexibility and compatibility.

coderabbitai · 2024-11-24T17:43:45Z

Caution

Review failed

The pull request is closed.

Walkthrough

The changes in this pull request primarily involve updates to the ESLint configuration, enhancements to error reporting and tracing functionalities, and modifications to the integration of Sentry and Honeybadger within the application. Key updates include the introduction of new rules in ESLint, the addition of helper methods for server rendering, and the restructuring of error reporting mechanisms. Furthermore, the package.json file has been revised to improve module exports and dependency management, while several files have been created or modified to support these integrations and functionalities.

Changes

File Path	Change Summary
`.eslintrc`	Updated ESLint rules: `no-shadow` set to `off`, `@typescript-eslint/no-shadow` set to `error`, and various TypeScript-specific configurations added.
`CHANGELOG.md`	Updated to include changes for version `4.0.0.rc.6`, detailing streaming server rendering support, new error handling configurations, and removal of Ruby 2.7 support.
`docs/node-renderer/error-reporting-and-tracing.md`	Title changed; new sections added for Sentry and Honeybadger integration instructions.
`package.json`	Introduced `exports` object, updated `peerDependencies`, added optional peer dependencies, and included new dependencies for Sentry testkits.
`packages/node-renderer/src/integrations/honeybadger.ts`	New file for integrating Honeybadger error reporting; includes `init` function.
`packages/node-renderer/src/integrations/sentry6.ts`	New file for Sentry integration; includes `init` function and extended interfaces for tracing.
`packages/node-renderer/src/master.ts`	Updated import for `errorReporter` to a namespace import and modified logging method for worker exits.
`packages/node-renderer/src/shared/configBuilder.ts`	Deprecated several properties related to error reporting in the `Config` interface.
`packages/node-renderer/src/shared/errorReporter.ts`	Refactored `ErrorReporter` class into standalone functions for message and error notifications.
`packages/node-renderer/src/shared/requireOptional.ts`	Deleted file that contained the `requireOptional` function.
`packages/node-renderer/src/shared/tracing.ts`	Removed `Tracing` class; introduced new interfaces and functions for tracing management.
`packages/node-renderer/src/shared/utils.ts`	Updated import for `errorReporter` to a namespace import and modified error reporting method.
`packages/node-renderer/src/worker.ts`	Modified import for `errorReporter` and updated error reporting and tracing function calls.
`packages/node-renderer/src/worker/handleRenderRequest.ts`	Updated import for `errorReporter` and changed error reporting function calls.
`packages/node-renderer/src/worker/vm.ts`	Updated import for `errorReporter` and changed error reporting method.
`packages/node-renderer/tests/shared/configBuilder.test.ts`	Deleted unit tests for the `buildConfig` function.
`packages/node-renderer/tests/shared/tracing.test.ts`	Updated tests for Sentry tracing; replaced `withinTransaction` with `trace` and simplified mocking.
`spec/dummy/client/node-renderer.js`	Integrated Honeybadger and Sentry for error tracking; removed related properties from the configuration object.
`packages/node-renderer/tests/htmlStreaming.test.js`	Updated import for `errorReporter` and modified error reporting assertions in tests.
`packages/node-renderer/src/integrations/sentry.ts`	New file for integrating Sentry error reporting; includes `init` function and tracing setup.
`spec/dummy/package.json`	Updated `@sentry/node` dependency version and removed `@sentry/tracing`.
`packages/node-renderer/src/integrations/api.ts`	New exports for error reporting and tracing functions and types.

Poem

In the burrows deep and wide,
Changes hop with joyful pride.
Sentry and Honeybadger sing,
Error reports on nimble wing.
With ESLint rules all aligned,
Our code is neat, our paths refined! 🐇✨

Note

🎁 Summarized by CodeRabbit Free

Your organization is on the Free plan. CodeRabbit will generate a high-level summary and a walkthrough for each pull request. For a comprehensive line-by-line review, please upgrade your subscription to CodeRabbit Pro by visiting https://app.coderabbit.ai/login.

🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

Review comments: Directly reply to a review comment made by CodeRabbit. Example:
- I pushed a fix in commit <commit_id>, please review it.
- Generate unit testing code for this file.
- Open a follow-up GitHub issue for this discussion.
Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
- @coderabbitai generate unit testing code for this file.
- @coderabbitai modularize this function.
PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
- @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
- @coderabbitai read src/utils.ts and generate unit testing code.
- @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
- @coderabbitai help me debug CodeRabbit configuration file.

Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments.

CodeRabbit Commands (Invoked using PR comments)

@coderabbitai pause to pause the reviews on a PR.
@coderabbitai resume to resume the paused reviews.
@coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
@coderabbitai full review to do a full review from scratch and review all the files again.
@coderabbitai summary to regenerate the summary of the PR.
@coderabbitai resolve resolve all the CodeRabbit review comments.
@coderabbitai configuration to show the current CodeRabbit configuration for the repository.
@coderabbitai help to get help.

Other keywords and placeholders

Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (`.coderabbit.yaml`)

You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
Please see the configuration documentation for more information.
If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

Visit our Documentation for detailed information on how to use CodeRabbit.
Join our Discord Community to get help, request features, and share feedback.
Follow us on X/Twitter for updates and announcements.

alexeyr-ci · 2024-11-25T12:40:48Z

    "@honeybadger-io/js": "^4.3.1",
-    "@sentry/node": "^6.2.3",
-    "@sentry/tracing": "^6.2.3",
+    "@sentry/node": "^7.120.0",


This version has both the new and the old tracing APIs, so both sentry6.ts and sentry.ts can be typed without additional tricks.

alexeyr-ci · 2024-11-25T12:49:54Z

@coderabbitai full review

coderabbitai · 2024-11-25T12:50:01Z

✅ Actions performed

Full review triggered.

alexeyr-ci · 2024-11-25T14:24:47Z

-const sentryTracing = requireOptional('@sentry/tracing');
+// This is the options necessary to start a unit of work (transaction/span/etc.).
+// Integrations should augment it using their name as the property.
+export interface UnitOfWorkOptions {}


Maybe StartTraceOptions would be clearer?

Make it clear about the types that's exist in the public interface. Explain how UnitOfWorkOptions should be created and passed to trace function. Also, explain that the TracingContext is passed to the callback function being executed when calling trace function.

AbanoubGhadban · 2024-11-26T11:50:02Z

-export = errorReporter;
+export function error(err: Error, tracingContext?: TracingContext) {
+  log.error(`ErrorReporter notification: ${err}`);
+  errorNotifiers.forEach((notifier) => {


Would it be better to use EventEmitter to notify listeners? It could simplify handling asynchronous operations. If you decide not to use it, consider executing the notifiers asynchronously like this:

errorNotifiers.forEach((notifier) => { setTimeout(() => notifier(err, tracingContext), 0); });

This approach ensures that a failure in one notifier doesn’t impact the caller of error or message. Additionally, it avoids slowing down the caller when there are many listeners. What do you think?

It looks like a good idea (maybe with a single setTimeout instead of corresponding to the number of notifiers). Need to check if it works properly with the Sentry 7 integration, though.

Fixed the possibility of notifiers failing, good catch!

Unfortunately, EventEmitter, setTimeout, and setImmediate all don't propagate AsyncLocalStorage in the way Sentry 7-8 need. I tried to work around it but didn't manage to. If I find a way later, it shouldn't be a breaking change.

AbanoubGhadban · 2024-11-26T12:38:20Z

+}
+
+export function init({ tracing = false } = {}) {
+  addMessageNotifier((msg) => {


You don't need to receive and pass the CaptureContext here, or is it missed?

No, the CaptureContext is needed for Sentry 6 to call setSpan for the captureMessage/captureException call. In Sentry 7-8, the active span is set automatically using AsyncLocalStorage.

AbanoubGhadban · 2024-11-26T12:38:34Z

+    captureMessage(msg);
+  });
+
+  addErrorNotifier((msg) => {


You don't need to receive and pass the CaptureContext here, or is it missed?

See #471 (comment).

AbanoubGhadban · 2024-11-26T12:50:17Z

+import * as Sentry from '@sentry/node';
+import sentryTestkit from 'sentry-testkit';
+import { trace } from '../../src/shared/tracing';
+import * as tracingIntegration from '../../src/integrations/sentry';


is it possible to add tests for sentry6 as well?

Not really, because we can only have one Sentry version in devDependencies. Now if we move it to a separate package, then we could.

Actually, let me see if I can make integrations into separate packages in the same repo...

AbanoubGhadban · 2024-11-26T12:54:18Z

+    }, testTransactionContext);
  }).rejects.toThrow();
-  expect(finishMock.mock.calls).toHaveLength(1);
+  expect(Sentry.getActiveSpan()).not.toBe(savedSpan);


Won't an error be reported automatically to Sentry if an error happens?

No, because toThrow catches it.

AbanoubGhadban · 2024-11-26T12:56:50Z

@@ -0,0 +1,7 @@
+export { addErrorNotifier, addMessageNotifier, addNotifier } from '../shared/errorReporter';


Please leave comment that tells this API can be used to develop new error reporting plugins for node renderer

Added with @file but it doesn't seem to show up anywhere useful...

alexeyr-ci force-pushed the alexeyr/integrations-as-plugins branch 10 times, most recently from 518aa0e to 49fc2b6 Compare November 25, 2024 12:19

alexeyr-ci mentioned this pull request Nov 25, 2024

Support Sentry SDK versions 7 to 8 #469

Closed

alexeyr-ci changed the title ~~[WIP] Convert error reporting and tracing integrations to plugins~~ Convert error reporting and tracing integrations to plugins Nov 25, 2024

alexeyr-ci force-pushed the alexeyr/integrations-as-plugins branch from 49fc2b6 to 76557be Compare November 25, 2024 12:27

alexeyr-ci commented Nov 25, 2024

View reviewed changes

Comment thread packages/node-renderer/src/integrations/index.ts Outdated

alexeyr-ci force-pushed the alexeyr/integrations-as-plugins branch 3 times, most recently from b46f7f5 to 5138288 Compare November 25, 2024 13:17

alexeyr-ci commented Nov 25, 2024

View reviewed changes

alexeyr-ci mentioned this pull request Nov 25, 2024

Allow correlating Ruby and JavaScript traces for Node renderer #473

Open

alexeyr-ci force-pushed the alexeyr/integrations-as-plugins branch from 9be5fd7 to 63aff64 Compare November 25, 2024 18:51

AbanoubGhadban reviewed Nov 26, 2024

View reviewed changes

alexeyr-ci force-pushed the alexeyr/integrations-as-plugins branch from 9e4a3f4 to b004b29 Compare November 27, 2024 09:20

alexeyr-ci merged commit dc36bba into master Nov 27, 2024

alexeyr-ci deleted the alexeyr/integrations-as-plugins branch November 27, 2024 09:20

alexeyr-ci mentioned this pull request Nov 27, 2024

Fire-and-forget notifier calls in ErrorReporter.notify #474

Open

claude Bot mentioned this pull request Feb 26, 2026

fix: replace marker file with Thor-idiomatic force option (#2289) shakacode/react_on_rails#2411

Merged

3 tasks

This was referenced May 4, 2026

feat: add REACT_ON_RAILS_BASE_PORT for concurrent worktree port management shakacode/react_on_rails#3142

Open

feat: rolling_deploy_adapter for seeding previous bundle hashes shakacode/react_on_rails#3173

Open

		@@ -0,0 +1,7 @@
		export { addErrorNotifier, addMessageNotifier, addNotifier } from '../shared/errorReporter';

Conversation

alexeyr-ci commented Nov 24, 2024 • edited by coderabbitai Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary by CodeRabbit

Release Notes

Uh oh!

coderabbitai Bot commented Nov 24, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review failed

Walkthrough

Changes

Poem

Chat

CodeRabbit Commands (Invoked using PR comments)

Other keywords and placeholders

CodeRabbit Configuration File (.coderabbit.yaml)

Documentation and Community

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alexeyr-ci commented Nov 25, 2024

Uh oh!

coderabbitai Bot commented Nov 25, 2024

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

AbanoubGhadban Nov 26, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

alexeyr-ci commented Nov 24, 2024 •

edited by coderabbitai Bot

Loading

coderabbitai Bot commented Nov 24, 2024 •

edited

Loading

CodeRabbit Configuration File (`.coderabbit.yaml`)

AbanoubGhadban Nov 26, 2024 •

edited

Loading