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: Error.prepareStackTrace method support #1180

Merged
merged 2 commits into from
Aug 9, 2019

Conversation

vtrifonov
Copy link
Contributor

Related to #1135

Added support for Error.prepareStackTrace method similar to the one in V8 and changed the format of the stack trace to be the same as android (also as node and browsers) with at ...

PR Checklist

@cla-bot cla-bot bot added the cla: yes label Aug 1, 2019
Changed the format of the stack trace to match node.js and V8
@vtrifonov vtrifonov force-pushed the trifonov/prepare-stack-trace branch 4 times, most recently from 7ecb60b to 27aab7b Compare August 6, 2019 11:00
@vtrifonov vtrifonov force-pushed the trifonov/prepare-stack-trace branch from 27aab7b to 1538eef Compare August 6, 2019 15:58
* prepareStackTrace cache
* added stack trace frame format test
* use StackFrame::formatStackFrame
@vtrifonov vtrifonov force-pushed the trifonov/prepare-stack-trace branch from a226305 to bfd5a9f Compare August 9, 2019 09:01
@mbektchiev mbektchiev merged commit 13699a4 into master Aug 9, 2019
@mbektchiev mbektchiev deleted the trifonov/prepare-stack-trace branch August 9, 2019 11:01
@mbektchiev mbektchiev added this to the 6.1.0 milestone Aug 9, 2019
vchimev added a commit to NativeScript/functional-tests that referenced this pull request Aug 13, 2019
@farfromrefug
Copy link

@vtrifonov @mbektchiev First thank for that PR!
Now i think we need to discuss a bit more on how to actually use this for the user.
It is not easy to actually make it all work. But i managed to end up with this:
Screen Shot 2019-08-23 at 14 18 08
The important thing is that i get real ts file absolute path. The great thing about this is that it allow ctrl+click to go the line/column. So useful in dev!

Now there are a few steps to make it work:

  • have webpack to generate source-map correctly:
    • you need to add options to config.output (it should work with source-map, inline-source-map ...)
devtoolModuleFilenameTemplate: info => {
                return info.absoluteResourcePath.split('?')[0];
            },
            devtoolFallbackModuleFilenameTemplate: info => {
                return info.absoluteResourcePath.split('?')[0];
            }
  • have your app correctly handle source maps. I do this only in dev by stripping the code with webpack in prod
const currentApp = knownFolders.currentApp();
process.cwd = function() {
    return '';
}
require('source-map-support').install({
    environment: 'node',
    handleUncaughtExceptions: false,
    retrieveSourceMap(source) {
        const sourceMapPath = source + '.map';
        const appPath = currentApp.path;
        let sourceMapRelativePath = sourceMapPath
            // .replace('file:///', '')
            .replace('file://', '')
            .replace(appPath + '/', '')
            .replace(appPath + '/', '');
        if (sourceMapRelativePath.startsWith('app/')) {
            sourceMapRelativePath = sourceMapRelativePath.slice(4);
        }
        return {
            url: sourceMapRelativePath,
            map: currentApp.getFile(sourceMapRelativePath).readTextSync()
        };
    }
});

Multiple things there:

  • i define process.cwd() because source-map-support requires it
  • i define retrieveSourceMap to recover the file when using source-map. there i need to cleanup the path to be able to read the file.

That should be good enough. For now i got this to work perfectly on Android.

Now some issues:

  • I dont seem to be able to make it work on iOS. any JS error will make the app crash with sig 11 and thus wont get in prepareStackTrace (using next runtime to have prepareStackTrace)
  • android: there is an issue with "native" errors where they can't be handled. not even sure they are going through prepareStackTrace as they are created on the native side. But the error that i get on the JS side has the JS stacktrace in "stacktrace" property instead of "stack".

Maybe we can discuss all that on slack. Would love to make it work consistently on iOS and Android. I could them publish a nativescript-source-map-support plugin.

Would it be possible for wepback templates to be updated to support this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants