fix: Don't send JavaScript objects across isolates

[timfish]

I found some intermittent test failures on some architectures.

I have diagnosed this as probably being caused by creating JavaScript objects in one isolate and returning them in another isolate.

This PR changes the module so it now it only passes strings between threads. It now returns a synthetic stack string for each thread which match the format of Node.js stack strings. This is probably preferable since this is less Sentry specific that the previous objects. We can also now pass these stack strings through the Sentry Node stack parser (as seen in the tests) and we get the additional in_app and module populated.

{
   '0': '    at from (node:buffer:299:28)\n' +
    '    at pbkdf2Sync (node:internal/crypto/pbkdf2:78:17)\n' +
    '    at longWork (/Users/tim/test/test/long-work.js:6:25)\n' +
    '    at ? (/Users/tim/test/test/stack-traces.js:11:1)\n' +
    '    at ? (node:internal/modules/cjs/loader:1734:14)\n' +
    '    at ? (node:internal/modules/cjs/loader:1899:10)\n' +
    '    at ? (node:internal/modules/cjs/loader:1469:32)\n' +
    '    at ? (node:internal/modules/cjs/loader:1286:12)\n' +
    '    at traceSync (node:diagnostics_channel:322:14)\n' +
    '    at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)\n' +
    '    at executeUserEntryPoint (node:internal/modules/run_main:152:5)\n' +
    '    at ? (node:internal/main/run_main_module:33:47)',
  '2': '    at from (node:buffer:299:28)\n' +
    '    at pbkdf2Sync (node:internal/crypto/pbkdf2:78:17)\n' +
    '    at longWork (/Users/tim/test/test/long-work.js:6:25)\n' +
    '    at ? (/Users/tim/test/test/worker.js:6:1)\n' +
    '    at ? (node:internal/modules/cjs/loader:1734:14)\n' +
    '    at ? (node:internal/modules/cjs/loader:1899:10)\n' +
    '    at ? (node:internal/modules/cjs/loader:1469:32)\n' +
    '    at ? (node:internal/modules/cjs/loader:1286:12)\n' +
    '    at traceSync (node:diagnostics_channel:322:14)\n' +
    '    at wrapModuleLoad (node:internal/modules/cjs/loader:235:24)\n' +
    '    at executeUserEntryPoint (node:internal/modules/run_main:152:5)\n' +
    '    at ? (node:internal/main/worker_thread:212:26)\n' +
    '    at [nodejs.internal.kHybridDispatch] (node:internal/event_target:827:20)\n' +
    '    at ? (node:internal/per_context/messageport:23:28)'
}

[AbhiPrasad]

@codecov-ai-reviewer review

[codecov-ai]

On it! We are reviewing the PR and will provide feedback shortly.

[codecov-ai]

PR Description

This pull request modifies the native stack trace capture functionality to return stack traces as formatted strings instead of structured objects. The primary goal is to align the stack trace format with the standard Node.js stack trace format, making it easier to integrate with existing tooling and libraries that expect this format.

Click to see more

Key Technical Changes

The core change is in module.cc, where the ExecutionInterrupted function now constructs a string representation of the stack trace instead of creating an array of JavaScript objects. The return type of CaptureStackTrace in module.cc and the corresponding TypeScript definition in src/index.ts have been updated to reflect this change. The tests in test/e2e.test.mjs have been updated to parse the new string-based stack traces using @sentry/core's stack parsing utilities.

Architecture Decisions

The decision to return string-based stack traces simplifies the native module's API and reduces the overhead of creating JavaScript objects in the C++ code. The adoption of @sentry/core for parsing the stack traces provides a standardized and well-tested approach for converting the string format back into structured data when needed.

Dependencies and Interactions

This pull request introduces a new dependency on the @sentry/core package. Consumers of this library will now need to ensure that @sentry/core is installed if they want to parse the stack trace strings into structured objects. The change also affects any existing code that relies on the previous structured stack trace format.

Risk Considerations

The primary risk is a breaking change for existing users of the library who are consuming the structured stack trace data. While the new string format is more standard, it requires users to update their code to parse the strings if they need structured data. There's also a risk that the string formatting in module.cc could introduce performance bottlenecks or platform-specific inconsistencies if not carefully implemented and tested.

Notable Implementation Details

The ExecutionInterrupted function in module.cc now iterates through the stack frames and constructs a string representation using std::string concatenation. The tests in test/e2e.test.mjs use @sentry/core's createStackParser and nodeStackLineParser to parse the string-based stack traces and verify their contents.

[timfish]

The primary goal is to align the stack trace format with the standard Node.js stack trace format

The primary goal is to fix the object corruption on some architectures which suggests something is not right.

I could create intermediate (non-v8) objects to safely pass between the threads and then build the javascript objects in the correct isolate. That way we could retain the existing API which returns StackFrame objects.

[timfish]

Closing in favour of #4