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: Add local source context via devserver #741

Merged
merged 3 commits into from
Jan 16, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ node_modules
build/
DerivedData
dist
coverage

## Various settings
*.pbxuser
Expand Down
9 changes: 6 additions & 3 deletions .travis.yml
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,8 @@ matrix:
# yarn: true
# script: .travis/run.sh

- language: android
- name: "Android"
language: android
sudo: required
jdk: oraclejdk8
before_cache:
Expand All @@ -40,7 +41,8 @@ matrix:
env: LANE='android'
script: .travis/run.sh

- language: objective-c
- name: "iOS"
language: objective-c
os: osx
osx_image: xcode9.4
node_js: 8
Expand All @@ -58,7 +60,8 @@ matrix:
- source ~/virtualenv/bin/activate
script: .travis/run.sh

- language: node_js
- name: "Deploy"
language: node_js
node_js: 8
script: .travis/deploy.sh
env: LANE='Deploy'
Expand Down
35 changes: 30 additions & 5 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
"build": "tsc -p tsconfig.build.json",
"build:watch": "tsc -p tsconfig.build.json -w --preserveWatchOutput",
"clean": "rimraf dist coverage",
"test": "jest",
"test:watch": "jest --watch",
"snyk-protect": "snyk protect",
"prepublish": "npm run snyk-protect"
},
Expand All @@ -32,20 +34,23 @@
"react-native": ">=0.56.0"
},
"dependencies": {
"@sentry/browser": "^5.10.0",
"@sentry/core": "^5.10.0",
"@sentry/integrations": "^5.10.0",
"@sentry/types": "^5.10.0",
"@sentry/utils": "^5.10.0",
"@sentry/browser": "^5.11.1",
"@sentry/core": "^5.11.1",
"@sentry/integrations": "^5.11.1",
"@sentry/types": "^5.11.0",
"@sentry/utils": "^5.11.1",
"@sentry/wizard": "^1.0.2"
},
"devDependencies": {
"@sentry/typescript": "5.10.0",
"@types/jest": "^24.0.25",
"@types/react-native": "^0.60.23",
"jest": "^24.9.0",
"prettier": "^1.19.1",
"replace-in-file": "^4.2.0",
"rimraf": "^3.0.0",
"snyk": "^1.256.0",
"ts-jest": "^24.3.0",
"tslint": "^5.20.1",
"typescript": "^3.4.5"
},
Expand All @@ -63,5 +68,25 @@
]
}
},
"jest": {
"collectCoverage": true,
"transform": {
"^.+\\.ts$": "ts-jest"
},
"moduleFileExtensions": [
"js",
"ts"
],
"testEnvironment": "node",
"testMatch": [
"**/*.test.ts"
],
"globals": {
"ts-jest": {
"tsConfig": "./tsconfig.json",
"diagnostics": false
}
}
},
"snyk": true
}
92 changes: 69 additions & 23 deletions src/js/integrations/debugsymbolicator.ts
Original file line number Diff line number Diff line change
@@ -1,12 +1,9 @@
import { addGlobalEventProcessor, getCurrentHub } from "@sentry/core";
import { Event, EventHint, Integration, StackFrame } from "@sentry/types";
import { logger } from "@sentry/utils";
import { addContextToFrame, logger } from "@sentry/utils";

const INTERNAL_CALLSITES_REGEX = new RegExp(
[
"/Libraries/Renderer/oss/ReactNativeRenderer-dev\\.js$",
"/Libraries/BatchedBridge/MessageQueue\\.js$"
].join("|")
["ReactNativeRenderer-dev\\.js$", "MessageQueue\\.js$"].join("|")
);

/**
Expand Down Expand Up @@ -70,7 +67,7 @@ export class DebugSymbolicator implements Integration {
await self._symbolicate(event, stack);
}
if (reactError.jsEngine === "hermes") {
const convertedFrames = this._convertReactNativeFramesToSentryFrames(
const convertedFrames = await this._convertReactNativeFramesToSentryFrames(
stack
);
this._replaceFramesInEvent(event, convertedFrames);
Expand Down Expand Up @@ -103,7 +100,7 @@ export class DebugSymbolicator implements Integration {
frame.file && frame.file.match(INTERNAL_CALLSITES_REGEX) === null
);

const symbolicatedFrames = this._convertReactNativeFramesToSentryFrames(
const symbolicatedFrames = await this._convertReactNativeFramesToSentryFrames(
stackWithoutInternalCallsites
);
this._replaceFramesInEvent(event, symbolicatedFrames);
Expand All @@ -121,25 +118,47 @@ export class DebugSymbolicator implements Integration {
* Converts ReactNativeFrames to frames in the Sentry format
* @param frames ReactNativeFrame[]
*/
private _convertReactNativeFramesToSentryFrames(
private async _convertReactNativeFramesToSentryFrames(
frames: ReactNativeFrame[]
): StackFrame[] {
): Promise<StackFrame[]> {
let getDevServer: any;
try {
if (__DEV__) {
getDevServer = require("react-native/Libraries/Core/Devtools/getDevServer");
}
} catch (_oO) {
// We can't load devserver URL
}
// Below you will find lines marked with :HACK to prevent showing errors in the sentry ui
// But since this is a debug only feature: This is Fine (TM)
return frames.map(
(frame: ReactNativeFrame): StackFrame => {
const inApp =
(frame.file && !frame.file.includes("node_modules")) ||
(!!frame.column && !!frame.lineNumber);
return {
colno: frame.column,
filename: frame.file,
function: frame.methodName,
in_app: inApp,
lineno: inApp ? frame.lineNumber : undefined, // :HACK
platform: inApp ? "javascript" : "node" // :HACK
};
}
return Promise.all(
frames.map(
async (frame: ReactNativeFrame): Promise<StackFrame> => {
let inApp = !!frame.column && !!frame.lineNumber;
inApp =
inApp &&
// tslint:disable-next-line: strict-type-predicates
frame.file !== undefined &&
!frame.file.includes("node_modules") &&
!frame.file.includes("native code");

const newFrame: StackFrame = {
colno: frame.column,
filename: frame.file,
function: frame.methodName,
in_app: inApp,
lineno: inApp ? frame.lineNumber : undefined, // :HACK
platform: inApp ? "javascript" : "node" // :HACK
Copy link
Member

@lobsterkatie lobsterkatie Jan 9, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that we're now filtering out both node modules and native code, would it be better to make the default undefined? Or to test for frame.file.includes("native code") also, and if it fails both the inApp test and the native code test, then have it default to node?

};

if (inApp && __DEV__) {
// tslint:disable-next-line: no-unsafe-any
await this._addSourceContext(newFrame, getDevServer);
}

return newFrame;
}
)
);
}

Expand All @@ -158,4 +177,31 @@ export class DebugSymbolicator implements Integration {
event.exception.values[0].stacktrace.frames = frames.reverse();
}
}

/**
* This tries to add source context for in_app Frames
*
* @param frame StackFrame
* @param getDevServer function from RN to get DevServer URL
*/
private async _addSourceContext(
frame: StackFrame,
getDevServer?: any
): Promise<void> {
// tslint:disable: no-unsafe-any no-non-null-assertion
const response = await fetch(
`${getDevServer().url}${frame
.filename!.replace(/\/+$/, "")
.replace(/.*\//, "")}`,
{
method: "GET"
}
);

const content = await response.text();
const lines = content.split("\n");

addContextToFrame(lines, frame);
// tslint:enable: no-unsafe-any no-non-null-assertion
}
}
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
"include": ["src/js/*.ts", "test/**/*.ts"],
"exclude": ["dist"],
"compilerOptions": {
"rootDir": "."
"rootDir": ".",
"types": ["jest"]
}
}
Loading