Skip to content

cleanup code #44

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.

Sign up for GitHub

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

Merged
merged 9 commits into from
Jun 5, 2019
Merged

cleanup code #44

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
218c464
cleanup code, update comments and readme, use URL API for repo extrac…
vanesa May 23, 2019
24b488b
Update README.md
vanesa May 24, 2019
c09fade
fix comments
vanesa May 24, 2019
bec2f21
catch errors when getting params from URI
vanesa May 30, 2019
efc8b4c
Update src/extension.ts
vanesa Jun 3, 2019
2317f1e
remove Params struct
vanesa Jun 3, 2019
4b76ad7
match any throw new TypeError()
vanesa Jun 3, 2019
c2e197c
wrap query in double quotes
vanesa Jun 3, 2019
345e593
update tests
vanesa Jun 3, 2019
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
22 changes: 11 additions & 11 deletions README.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,16 +37,16 @@ Set the following configurations in your settings:
"projectId": "[Sentry project ID, e.g. "1334031"]",
"linePatterns": [
// List of RegExp patterns that match error handling code, e.g. "throw new Error+\\(['\"]([^'\"]+)['\"]\\)",
// !! Make sure to capture the error message in a RegExp group !!
// !! Make sure to capture the error message in a RegExp group and escape special characters to ensure compatibility with JSON Schema!!
Copy link
Contributor

@felixfbecker felixfbecker Jun 5, 2019

Choose a reason for hiding this comment

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

I'd drop the "Schema" because you need to escape to be compliant with JSON (not with the schema)
The exclamation marks seem very aggressive

]
"filters": {
[
"repositories": [
// List of RegExp repo names asociated with this Sentry project
// List of RegExp repo names asociated with this Sentry project.
],
"files": [
// List of RegExp that matches file format, e.g. "\\.tsx?",
// or for more specific matching, folder matching, e.g. "(?:web|shared|src)\/.*\\.tsx?"
// or for more specific matching, folder matching, e.g. "(?:web|shared|src)/.*\\.tsx?"
],
}
}
Expand All @@ -59,7 +59,7 @@ File patterns can also be narrowed down to certain folders by specifying this in

```
...
"files": ["(?:web|shared|src)\/.*\\.tsx?"]
"files": ["(?:web|shared|src)/.*\\.tsx?"]
...
```

Expand All @@ -78,15 +78,15 @@ File patterns can also be narrowed down to certain folders by specifying this in
"projectId": "1334031",
"linePatterns": [
"throw new Error+\\(['\"]([^'\"]+)['\"]\\)",
"console\\.(warn|debug|info|error)\\(['\"`]([^'\"`]+)['\"`]\\)"
"console\\.(?:warn|debug|info|error)\\(['\"`]([^'\"`]+)['\"`]\\)"
]
"filters": [
{
"repositories": "sourcegraph\/sourcegraph",
"files": ["web\/.*\\.ts?"],
"repositories": "sourcegraph/sourcegraph",
"files": ["web/.*\\.ts?"],
},
{
"files": ["sourcegraph-subfolder\/.*\\.tsx?"]
"files": ["sourcegraph-subfolder/.*\\.tsx?"]
}

]
Expand All @@ -111,11 +111,11 @@ Configuration:
"linePatterns": ["errors\\.New\\(['\"`](.*)['\"`]\\)"],
"filters": [
{
"repositories": ["sourcegraph\/sourcegraph", "sourcegraph\/dev-repo"],
"files": ["/auth\/.*.go?/"],
"repositories": ["sourcegraph/sourcegraph", "sourcegraph/dev-repo"],
"files": ["auth/.*\\.go?"],
},
{
"repositories": ["/dev-env/"]
"repositories": ["/dev-env"]
}
],
]
Expand Down
15 changes: 4 additions & 11 deletions src/extension.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,13 @@ import * as sourcegraph from 'sourcegraph'
import { createDecoration, getParamsFromUriPath, matchSentryProject } from './handler'
import { resolveSettings, SentryProject, Settings } from './settings'

/**
* Params derived from the document's URI.
*/
interface Params {
repo: string | null
file: string | null
}

/**
* Common error log patterns to use in case no line matching regexes
* are set in the sentry extension settings.
*/
const COMMON_ERRORLOG_PATTERNS = [
// typescript/javascript
/throw new Error+\(['"]([^'"]+)['"]\)/gi,
/throw new ([A-Z][a-z]+)+\(['"]([^'"]+)['"]\)/gi,
/console\.(error|info|warn)\(['"`]([^'"`]+)['"`]\)/gi,
// go
/log\.(Printf|Print|Println)\(['"]([^'"]+)['"]\)/gi,
Expand Down Expand Up @@ -84,7 +76,7 @@ export function getDecorations(
documentText: string,
sentryProjects?: SentryProject[]
): sourcegraph.TextDocumentDecoration[] {
const params: Params = getParamsFromUriPath(documentUri)
const params = getParamsFromUriPath(documentUri)
const matched = sentryProjects && matchSentryProject(params, sentryProjects)

// Do not decorate lines if the document file format does not match the
Expand Down Expand Up @@ -189,7 +181,8 @@ function buildUrl(errorQuery: string, sentryProjectId?: string): URL {

if (sentryProjectId) {
url.searchParams.set('project', sentryProjectId)
url.searchParams.set('query', 'is:unresolved ' + errorQuery)
// Query must be wrapped in double quotes to be used as a search query in Sentry
url.searchParams.set('query', 'is:unresolved ' + '"' + errorQuery + '"')
url.searchParams.set('statsPeriod', '14d')
}

Expand Down
21 changes: 12 additions & 9 deletions src/handler.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,15 +17,18 @@ export interface LineDecorationText {
* @param textDocumentURI A document URI.
* @returns repo and file part of URI.
*/
export function getParamsFromUriPath(textDocumentURI: string): Params {
// TODO: Support more than just GitHub & Gitlab.
// TODO: Safeguard for cases where repo/fileMatch are null.
const repoPattern = /(github\.com|gitlab\.com)\/([^\?\#\/]+\/[^\?\#\/]*)/gi
export function getParamsFromUriPath(textDocumentURI: string): Params | null {
let paramsRepo
let fileMatch
Copy link
Contributor

@felixfbecker felixfbecker Jun 5, 2019

Choose a reason for hiding this comment

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

Declare a type (else they are any)

const filePattern = /#(.*\.(.*))$/gi
const repoMatch = repoPattern.exec(textDocumentURI)
const fileMatch = filePattern.exec(textDocumentURI)
try {
paramsRepo = new URL(textDocumentURI).pathname
fileMatch = filePattern.exec(textDocumentURI)
} catch (err) {
return null
}
return {
repo: repoMatch && repoMatch[2],
repo: paramsRepo,
file: fileMatch && fileMatch[1],
}
}
Expand All @@ -42,8 +45,8 @@ interface Matched {
* @param projects Sentry extension projects configurations.
* @return Sentry projectID this document reports to.
*/
export function matchSentryProject(params: Params, projects: SentryProject[]): Matched | null {
if (!projects || !params.repo || !params.file) {
export function matchSentryProject(params: Params | null, projects: SentryProject[]): Matched | null {
if (!projects || !params || !params.repo || !params.file) {
return null
}
// Check if a Sentry project is associated with this document's repository and/or file and retrieve the project.
Expand Down
60 changes: 29 additions & 31 deletions src/test/extension.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,33 +16,31 @@ describe('activation', () => {
})
})

const asString = (re: RegExp): string => re.source

const projects: SentryProject[] = [
{
name: 'Webapp typescript errors',
projectId: '1334031',
linePatterns: [
/throw new Error+\(['"]([^'"]+)['"]\)/,
/console\.(warn|debug|info|error|log)\(['"`]([^'"`]+)['"`]\)/,
/log\.(Printf|Print|Println)\(['"]([^'"]+)['"]\)/,
].map(asString),
'throw new Error+\\([\'"]([^\'"]+)[\'"]\\)',
'console\\.(?:warn|debug|info|error|log)\\([\'"`]([^\'"`]+)[\'"`]\\)',
'log\\.(?:Printf|Print|Println)\\([\'"]([^\'"]+)[\'"]\\)',
],
filters: [
{
repositories: [/sourcegraph\/sourcegraph/, /bucket/].map(asString),
files: [/(web|shared|src)\/.*\.tsx?/, /\/.*\\.ts?/].map(asString),
repositories: ['sourcegraph/sourcegraph', '/bucket'],
files: ['(?:web|shared|src)/.*\\.tsx?', '\\.ts?'],
},
],
},

{
name: 'Dev env errors',
projectId: '213332',
linePatterns: [/log\.(Printf|Print|Println)\(['"]([^'"]+)['"]\)/].map(asString),
linePatterns: ['log\\.(Printf|Print|Println)\\([\'"]([^\'"]+)[\'"]\\)'],
filters: [
{
repositories: [/dev-repo/].map(asString),
files: [/(dev)\/.*\\.go?/].map(asString),
repositories: ['/dev-repo'],
files: ['dev/.*\\.go?'],
},
],
},
Expand All @@ -64,25 +62,25 @@ describe('resolveSettings()', () => {
projectId: '1334031',
name: 'Webapp typescript errors',
linePatterns: [
/throw new Error+\(['"]([^'"]+)['"]\)/,
/console\.(warn|debug|info|error|log)\(['"`]([^'"`]+)['"`]\)/,
/log\.(Printf|Print|Println)\(['"]([^'"]+)['"]\)/,
].map(asString),
'throw new Error+\\([\'"]([^\'"]+)[\'"]\\)',
'console\\.(?:warn|debug|info|error|log)\\([\'"`]([^\'"`]+)[\'"`]\\)',
'log\\.(?:Printf|Print|Println)\\([\'"]([^\'"]+)[\'"]\\)',
],
filters: [
{
repositories: [/sourcegraph\/sourcegraph/, /bucket/].map(asString),
files: [/(web|shared|src)\/.*\.tsx?/, /\/.*\\.ts?/].map(asString),
repositories: ['sourcegraph/sourcegraph', '/bucket'],
files: ['(?:web|shared|src)/.*\\.tsx?', '\\.ts?'],
},
],
},
{
projectId: '213332',
name: 'Dev env errors',
linePatterns: [/log\.(Printf|Print|Println)\(['"]([^'"]+)['"]\)/].map(asString),
linePatterns: ['log\\.(Printf|Print|Println)\\([\'"]([^\'"]+)[\'"]\\)'],
filters: [
{
repositories: [/dev-repo/].map(asString),
files: [/(dev)\/.*\\.go?/].map(asString),
repositories: ['/dev-repo'],
files: ['dev/.*\\.go?'],
},
],
},
Expand All @@ -108,7 +106,7 @@ const decorateLineInput = [
contentText: ' View logs in Sentry » ',
hoverMessage: ' View logs in Sentry » ',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=134412&query=is%3Aunresolved+cannot+determine+file+path&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=134412&query=is%3Aunresolved+%22cannot+determine+file+path%22&statsPeriod=14d',
},
},
},
Expand All @@ -127,7 +125,7 @@ const decorateLineInput = [
contentText: ' View logs in Sentry (❕)» ',
hoverMessage: ' Add this repository to your Sentry extension settings for project matching.',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=134412&query=is%3Aunresolved+cannot+determine+file+path&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=134412&query=is%3Aunresolved+%22cannot+determine+file+path%22&statsPeriod=14d',
},
},
},
Expand All @@ -146,7 +144,7 @@ const decorateLineInput = [
contentText: ' View logs in Sentry (❕)» ',
hoverMessage: ' Add this repository to your Sentry extension settings for project matching.',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=134412&query=is%3Aunresolved+cannot+determine+file+path&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=134412&query=is%3Aunresolved+%22cannot+determine+file+path%22&statsPeriod=14d',
},
},
},
Expand Down Expand Up @@ -226,7 +224,7 @@ const getDecorationsInput = [
contentText: ' View logs in Sentry » ',
hoverMessage: ' View logs in Sentry » ',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+cannot+determine+file+path&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+%22cannot+determine+file+path%22&statsPeriod=14d',
},
},
{
Expand All @@ -238,7 +236,7 @@ const getDecorationsInput = [
contentText: ' View logs in Sentry » ',
hoverMessage: ' View logs in Sentry » ',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+cannot+determine+delta+info&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+%22cannot+determine+delta+info%22&statsPeriod=14d',
},
},
],
Expand All @@ -262,7 +260,7 @@ const getDecorationsInput = [
contentText: ' View logs in Sentry » ',
hoverMessage: ' View logs in Sentry » ',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+cannot+determine+file+path&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+%22cannot+determine+file+path%22&statsPeriod=14d',
},
},
],
Expand Down Expand Up @@ -316,7 +314,7 @@ of(codeView).pipe(
contentText: ' View logs in Sentry » ',
hoverMessage: ' View logs in Sentry » ',
linkURL:
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+cannot+determine+file+path&statsPeriod=14d',
'https://sentry.io/organizations/sourcegraph/issues/?project=1334031&query=is%3Aunresolved+%22cannot+determine+file+path%22&statsPeriod=14d',
},
},
],
Expand Down Expand Up @@ -460,8 +458,8 @@ describe('buildDecorations()', () => {
it('should not render anything due to missing code ', () => expect(buildDecorations([], '')).toEqual([]))
// set linePatterns back to original state for the other tests
projects[0].linePatterns = [
/throw new Error+\(['"]([^'"]+)['"]\)/,
/console\.(warn|debug|info|error|log)\(['"`]([^'"`]+)['"`]\)/,
/log\.(Printf|Print|Println)\(['"]([^'"]+)['"]\)/,
].map(asString)
'throw new Error+\\([\'"]([^\'"]+)[\'"]\\)',
'console\\.(?:warn|debug|info|error|log)\\([\'"`]([^\'"`]+)[\'"`]\\)',
'log\\.(?:Printf|Print|Println)\\([\'"]([^\'"]+)[\'"]\\)',
]
})
Loading