Display Bundle Errors in debug console #964

filip131311 · 2025-02-12T20:31:20Z

This PR displays bundle errors in debug console and focuses on the error source, because some bundle errors happen before the application is activated and therefore the cdp debugger we rework how the debug session is created, we first create just a shell of debugSession, that is not connected to the debugger, but is able to log messages into debug console and latter when the application is ready we connect the debugSession to it.

Because this new pattern introduces a possibility of DAP methods being called before debugAdapter is connected to cdp we move cdp related functionality to debug session and return empty responses in case it does not exist yet.

How Has This Been Tested:

-run react-native-78 test app and force bundle error also check if the app is working as expected when syntax is correct.

Screen.Recording.2025-02-19.at.13.55.51.mov

vercel · 2025-02-12T20:31:25Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Comments	Updated (UTC)
radon-ide	✅ Ready (Inspect)	Visit Preview	💬 Add feedback	Feb 27, 2025 11:41am

kmagiera

Left some comment inline but also wonder if we could:

We should avoid code where we treat cdpSession as optional, with updates to cdpSession? or cdpSession! – they are perhaps mostly good for now, but when this is a common pattern across this file it may lead to situations where we silently ignore certain error states. I think the best solution would be to try and extract as much code to CDPSession or have some in-between class that could rely on the session being always present. The current change introduces a critical yet implicit state where this class can function with and without sessions and have different behavior.

I wonder how debug session initialization is handled after this change. It appears like this PR is not changing that, therefore the debug adapter will get initialized only after the JS debugger is connected. As a consequence I wonder if and how it will work in the following scenarios:

Can we trigger pause events / errors before debug adapter sends InitializedEvent
Can we land in a state when debug adapter is re-used and re-connected after restart. For example when JS debugger is connected, app crashes in runtime disconnecting the debugger and we hit reload?

packages/vscode-extension/src/project/project.ts

packages/vscode-extension/src/debugging/DebugSession.ts

filip131311 · 2025-02-19T12:32:13Z

@kmagiera

Can we trigger pause events / errors before debug adapter sends InitializedEvent?

I reworked the approach to focusing and use window.showTextDocument to avoid faking pause events as this solution was not stable enough.

Can we land in a state when debug adapter is re-used and re-connected after restart. For example when JS debugger is connected, app crashes in runtime disconnecting the debugger and we hit reload?

No this is prevented by:

if (this.wasConnectedToCDP) {
      this.vscSession && debug.stopDebugging(this.vscSession);
      await this.startDebugSession();
    }

kmagiera

I wonder if it would be possible to start the main JS debugger as child session and have the metro DebugAdapter as a separate concept. We could use debug.startDebugging parentSession parameter for that. Maybe, when it is setup this way the session would share the console? Idk

filip131311 · 2025-02-25T14:30:28Z

@kmagiera I tested the approach with a parent debugger, with de following code:

const parentDebugger = await debug.startDebugging( undefined,
      {
        type: "com.swmansion.react-native-debugger",
        name: "Radon IDE Debugger 22",
        request: "attach",
      },
      {
        suppressDebugStatusbar: true,
        suppressDebugView: true,
        suppressDebugToolbar: true,
        suppressSaveBeforeStart: true,
      });
    const activeDebugSession = debug.activeDebugSession;
    const debugStarted = await debug.startDebugging(
      undefined,
      {
        type: "com.swmansion.react-native-debugger",
        name: "Radon IDE Debugger",
        request: "attach",
      },
      {
        parentSession: activeDebugSession ,
        suppressDebugStatusbar: true,
        suppressDebugView: true,
        suppressDebugToolbar: true,
        suppressSaveBeforeStart: true,
      }
    );

    if (debugStarted) {
      this.vscSession = debug.activeDebugSession!;
.
.
.

and unfortunatally the result was the same as when we didn't use parent debugger configuration. Both debuggers were still selectable by a user, which introduces a risk of the wrong one being selected, and whats worse would split the output of bundle errors and application logs.

kmagiera

Left a couple more comments inline. Also, I wonder if this change won't conflict with #985 (I imagine it would, but I'd hope we can resolve it rather easily)

kmagiera · 2025-02-26T10:15:08Z

packages/vscode-extension/src/common/Project.ts

    | "bundleError"
-    | "incrementalBundleError"
+    | "bundlingError"


we can't have both bundleError and bundlingError, how do you tell which one means what?

kmagiera · 2025-02-26T10:44:44Z

packages/vscode-extension/src/debugging/DebugSession.ts

+    await this.session.customRequest("RNIDE_connect_cdp_debugger", cdpConfiguration);
+  }
+
+  public async sendDebugConsoleLog(message: string, source?: DebugSource) {


"send" doesn't feel right as we are not sending logs. This should rather be "append". Also I believe we should actually report this as "error" and not "log". So maybe this should be appendDebuggerConsoleEntry or writeToDebuggerConsole and the method should also take the category of the entry (log/warning/error etc)

kmagiera · 2025-02-26T10:52:59Z

packages/vscode-extension/src/debugging/DebugSession.ts

-      });
-    }
-
+  public async startDebugSession() {


we are mixing up names here which makes this change very confusing. Sometime we refer to the connection with JS debugger as JSDebugger and at other times we use DebugSession. I think we should just keep using start here and in places where we explicitely deal with the CDP JS debugger, we should use JSDebugger

kmagiera · 2025-02-26T10:53:53Z

packages/vscode-extension/src/project/deviceSession.ts

+    this.debugSession = new DebugSession(this.debugEventDelegate);
+    this.debugSession.startDebugSession();


We should avoid exposing too many unnecessary implementation details. If this isn't useful to have separate control over creating and starting, the constructor should just start the session on its own.

kmagiera · 2025-02-26T10:55:52Z

packages/vscode-extension/src/project/deviceSession.ts

+      await this.debugSession.stopDebugSession();
+      await this.debugSession.startDebugSession();


Similatily here, I understand that we are referring to JS debugger here. We don't need to leak the implementation details here. If we want to have a signal that the session needs to be cleared, we should expose a mathod for that and the session on its own will decide that it should restart the JS debugger connection.

kmagiera · 2025-02-26T10:56:34Z

packages/vscode-extension/src/project/deviceSession.ts

-    this.debugSession = new DebugSession(this.metro, this.debugEventDelegate);
-    const started = await this.debugSession.start();
-    if (started) {
+  private async connectDebugger() {


Lets be consistent and use JSDebugger here. We have too many things called debugger or debug session already.

init

78d7a41

kmagiera requested changes Feb 14, 2025

View reviewed changes

filip131311 added 2 commits February 18, 2025 15:23

first cr comment incremental build name

c57b2f1

focus on the error source by heand instead of triggering pauseEvent

c1fad65

vercel bot deployed to Preview February 18, 2025 15:39 View deployment

detect file name in more cases

933bbcc

vercel bot deployed to Preview February 18, 2025 16:06 View deployment

remove dead code

97721d1

vercel bot deployed to Preview February 18, 2025 16:15 View deployment

marge main

120bbc5

vercel bot deployed to Preview February 18, 2025 16:17 View deployment

move cdp related logic from debug addpater to cdpSession

e95bb7f

vercel bot deployed to Preview February 19, 2025 12:19 View deployment

formatting

ce0b835

vercel bot deployed to Preview February 19, 2025 12:27 View deployment

clean up

392f762

vercel bot deployed to Preview February 19, 2025 12:55 View deployment

filip131311 requested a review from kmagiera February 19, 2025 12:56

kmagiera reviewed Feb 21, 2025

View reviewed changes

resolve conflicts

405b92e

vercel bot deployed to Preview February 25, 2025 14:32 View deployment

filip131311 requested a review from kmagiera February 25, 2025 14:32

kmagiera requested changes Feb 26, 2025

View reviewed changes

filip131311 added 3 commits February 27, 2025 11:27

Merge branch 'main' into Filip131311/DisplayBundlerErrorsInDebugConsole

2abcdd5

CR

7d73826

CR

d2bb63a

vercel bot deployed to Preview February 27, 2025 11:41 View deployment

filip131311 requested a review from kmagiera February 27, 2025 19:38

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Display Bundle Errors in debug console #964

Display Bundle Errors in debug console #964

filip131311 commented Feb 12, 2025 •

edited

Loading

vercel bot commented Feb 12, 2025 •

edited

Loading

kmagiera left a comment

filip131311 commented Feb 19, 2025

kmagiera left a comment

filip131311 commented Feb 25, 2025

kmagiera left a comment

kmagiera Feb 26, 2025

kmagiera Feb 26, 2025

kmagiera Feb 26, 2025

kmagiera Feb 26, 2025

kmagiera Feb 26, 2025

kmagiera Feb 26, 2025

		this.debugSession = new DebugSession(this.debugEventDelegate);
		this.debugSession.startDebugSession();

		await this.debugSession.stopDebugSession();
		await this.debugSession.startDebugSession();

Display Bundle Errors in debug console #964

Are you sure you want to change the base?

Display Bundle Errors in debug console #964

Conversation

filip131311 commented Feb 12, 2025 • edited Loading

How Has This Been Tested:

vercel bot commented Feb 12, 2025 • edited Loading

kmagiera left a comment

Choose a reason for hiding this comment

filip131311 commented Feb 19, 2025

kmagiera left a comment

Choose a reason for hiding this comment

filip131311 commented Feb 25, 2025

kmagiera left a comment

Choose a reason for hiding this comment

kmagiera Feb 26, 2025

Choose a reason for hiding this comment

kmagiera Feb 26, 2025

Choose a reason for hiding this comment

kmagiera Feb 26, 2025

Choose a reason for hiding this comment

kmagiera Feb 26, 2025

Choose a reason for hiding this comment

kmagiera Feb 26, 2025

Choose a reason for hiding this comment

kmagiera Feb 26, 2025

Choose a reason for hiding this comment

filip131311 commented Feb 12, 2025 •

edited

Loading

vercel bot commented Feb 12, 2025 •

edited

Loading