Skip to content

Commit 948fab9

Browse files
authored
Sentry report improvements (#330)
* [sentry][m]: promisified sending reports to Sentry - refs #329 This is necessary as we want to process.exit once report is sent but we also don't want rest of the code to run if an error occured. To make it work, we've decided to promisify the report sending part of the 'handleError' function and await until it finishes from the caller. This commit also includes changes in `/bin/` modules to work with refactored `handleError` function. * [validate][xs]: don't use 'handleError' function as it is tableschema error so we handle it withing the module - refs #329 * [Sentry][xs]: include all arguments into a report - refs #329 Also sending report before printing errors so it has better UX * [Sentry][xs]: include user's OS in the report - refs #329
1 parent 62f6a8d commit 948fab9

File tree

9 files changed

+47
-27
lines changed

9 files changed

+47
-27
lines changed

bin/data-cat.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -54,7 +54,8 @@ const dumpIt = async (res, {sheet}={}) => {
5454
if (isUrl(argv._[0])) {
5555
error('Provided URL is invalid')
5656
}
57-
handleError(err)
57+
await handleError(err)
58+
process.exit(1)
5859
}
5960

6061
if (outFormat === 'ascii') { // Write to stdout

bin/data-get.js

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ const run = async () => {
8181
if (!checkDestIsEmpty(owner, name)) {
8282
throw new Error(`${owner}/${name} is not empty!`)
8383
}
84-
84+
8585
/** For datasets from the datahub we get zipped version and unzip it.
8686
- less traffic
8787
- zipped version has a fancy file structure
@@ -112,7 +112,8 @@ const run = async () => {
112112
if (argv.debug) {
113113
console.log('> [debug]\n' + err.stack)
114114
}
115-
handleError(err)
115+
await handleError(err)
116+
process.exit(1)
116117
}
117118
}
118119

@@ -137,7 +138,8 @@ const saveFileFromUrl = (url, format) => {
137138
if (err.message === 'Not Found') {
138139
err.message += ' or Forbidden.'
139140
}
140-
handleError(err)
141+
await handleError(err)
142+
process.exit(1)
141143
}
142144
stream.pipe(fs.createWriteStream(destPath)).on('finish', () => {
143145
resolve(destPath)

bin/data-info.js

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -69,6 +69,7 @@ Promise.resolve().then(async () => {
6969
if (!argv._[0]) {
7070
printInfo('Running `data info` without an argument will search a `datapackage.json` file in the current working directory.')
7171
}
72-
handleError(err)
72+
await handleError(err)
73+
process.exit(1)
7374
}
7475
})

bin/data-login.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,8 @@ Promise.resolve().then(async () => {
3737
try {
3838
out = await authenticate(apiUrl, token)
3939
} catch (err) {
40-
handleError(err)
40+
await handleError(err)
41+
process.exit(1)
4142
}
4243
if (out.authenticated) {
4344
stopSpinner()
@@ -67,7 +68,8 @@ Promise.resolve().then(async () => {
6768
try {
6869
await login(apiUrl, authUrl, config.get('domain'))
6970
} catch (err) {
70-
handleError(err)
71+
await handleError(err)
72+
process.exit(1)
7173
}
7274
info('You are logged in!')
7375
process.exit(0)

bin/data-push-flow.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -41,7 +41,8 @@ Promise.resolve().then(async () => {
4141
try {
4242
out = await authenticate(apiUrl, token)
4343
} catch (err) {
44-
handleError(err)
44+
await handleError(err)
45+
process.exit(1)
4546
}
4647
if (!out.authenticated) {
4748
info('You need to login in order to push your data. Please, use `data login` command.')
@@ -75,6 +76,7 @@ Promise.resolve().then(async () => {
7576
if (argv.debug) {
7677
console.log('> [debug]\n' + err.stack)
7778
}
78-
handleError(err)
79+
await handleError(err)
80+
process.exit(1)
7981
}
8082
})

bin/data-push.js

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,8 @@ Promise.resolve().then(async () => {
4747
try {
4848
out = await authenticate(apiUrl, token)
4949
} catch (err) {
50-
handleError(err)
50+
await handleError(err)
51+
process.exit(1)
5152
}
5253
}
5354
if (!out.authenticated) {
@@ -69,7 +70,8 @@ Promise.resolve().then(async () => {
6970
info("'data validate' to check your data.")
7071
info("'data init' to create a datapackage.")
7172
info("'data help push' to get more info.")
72-
handleError(err)
73+
await handleError(err)
74+
process.exit(1)
7375
}
7476
} else {
7577
dataset = await prepareDatasetFromFile(filePath)
@@ -137,7 +139,8 @@ Promise.resolve().then(async () => {
137139
if (argv.debug) {
138140
console.log('> [debug]\n' + err.stack)
139141
}
140-
handleError(err)
142+
await handleError(err)
143+
process.exit(1)
141144
}
142145
})
143146

bin/data-validate.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@ const {eraseLines} = require('ansi-escapes')
1111

1212
// Ours
1313
const {customMarked} = require('../lib/utils/tools')
14-
const {handleError, error} = require('../lib/utils/error')
14+
const {error} = require('../lib/utils/error')
1515
const wait = require('../lib/utils/output/wait')
1616
const info = require('../lib/utils/output/info')
1717

@@ -71,7 +71,11 @@ validator.validate().then(result => {
7171
})
7272
}
7373
else {
74-
handleError(result)
74+
if (result.constructor.name === 'Array') {
75+
result.forEach(err => error(err.message))
76+
} else {
77+
error(result)
78+
}
7579
}
7680
}
7781
}).catch(err => {

bin/data.js

Lines changed: 6 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,14 @@ const updateNotifier = require('../lib/utils/update')
1919
require('events').EventEmitter.defaultMaxListeners = 120;
2020

2121
// Handle all uncaught exceptions and unhandled rejections
22-
process.on('uncaughtException', (err) => {
23-
handleError(err)
22+
process.on('uncaughtException', async (err) => {
23+
await handleError(err)
24+
process.exit(1)
2425
})
2526

26-
process.on('unhandledRejection', (err) => {
27-
handleError(err)
27+
process.on('unhandledRejection', async (err) => {
28+
await handleError(err)
29+
process.exit(1)
2830
})
2931

3032
// Check and notify if any updates are available:

lib/utils/error.js

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -7,25 +7,28 @@ const {version} = require('../../package.json')
77
const {installedWithNPM} = require('./tools')
88

99

10-
function handleError(err, {debug = false} = {}) {
10+
async function handleError(err, {debug = false} = {}) {
1111
if (process.env.datahub !== 'dev') { // Send report to Sentry if not dev env
1212
// Setup Sentry:
1313
Raven.config('https://e29902aa81ed414d867f51bd0d1ab91a:[email protected]/305079', {
1414
release: version,
1515
extra: {
16-
firstArg: process.argv.slice(2, 3),
17-
rest: process.argv.slice(3, process.argv.length),
18-
nodejsOrBin: installedWithNPM ? process.version : 'bin'
16+
args: process.argv,
17+
nodejsOrBin: installedWithNPM ? process.version : 'bin',
18+
os: process.platform
1919
}
2020
})
2121

22-
// Capture errors:
23-
Raven.captureException(err, (sendErr, eventId) => {
24-
// Once report is sent exit with code 1:
25-
process.exit(1)
22+
await new Promise((resolve, reject) => {
23+
// Capture errors:
24+
Raven.captureException(err, (sendErr, eventId) => {
25+
// Once report is sent resolve the promise. However, we resolve it even
26+
// if it failed to send a report:
27+
resolve()
28+
})
2629
})
2730
}
28-
31+
2932
// Coerce Strings to Error instances
3033
if (typeof err === 'string') {
3134
err = new Error(err)

0 commit comments

Comments
 (0)