Skip to content

Commit

Permalink
Revert "support blocking unhandled errors with --abort-on-uncaught-ex…
Browse files Browse the repository at this point in the history
…ception"

This reverts commit 5323576.
  • Loading branch information
uurien committed Jun 25, 2024
1 parent cb6fd05 commit 52a7f2c
Show file tree
Hide file tree
Showing 5 changed files with 4 additions and 118 deletions.
26 changes: 2 additions & 24 deletions integration-tests/appsec/index.spec.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ describe('RASP', () => {
}

const confs = [
{ abortOnUncaughtException: true },
// { abortOnUncaughtException: true },
{ abortOnUncaughtException: false }]
// execArgv --abort-on-uncaught-exception

Expand Down Expand Up @@ -111,29 +111,6 @@ describe('RASP', () => {
}
})

it('should crash when promise rejection is not an AbortError', async () => {
let hasOutput = false
stdioHandler = () => {
hasOutput = true
}

try {
await axios.get('/crash-promise')

assert.fail('Request should have failed')
} catch (e) {
return new Promise((resolve, reject) => {
setTimeout(() => {
if (hasOutput) {
resolve()
} else {
reject(new Error('Output expected after crash'))
}
}, 50)
})
}
})

it('should not crash when customer has his own setUncaughtExceptionCaptureCallback', async () => {
let hasOutput = false
stdioHandler = () => {
Expand Down Expand Up @@ -170,6 +147,7 @@ describe('RASP', () => {

assert.fail('Request should have failed')
} catch (e) {
// console.log(e)
return new Promise((resolve, reject) => {
setTimeout(() => {
if (hasOutput) {
Expand Down
6 changes: 0 additions & 6 deletions integration-tests/appsec/rasp/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,12 +50,6 @@ app.get('/crash', () => {
})
})

app.get('/crash-promise', () => {
process.nextTick(() => {
Promise.reject(new Error('Crash'))
})
})

app.get('/crash-and-recovery-A', (req, res) => {
process.setUncaughtExceptionCaptureCallback(() => {
res.writeHead(500)
Expand Down
3 changes: 0 additions & 3 deletions packages/datadog-instrumentations/src/helpers/register.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,9 +27,6 @@ const loadChannel = channel('dd-trace:instrumentation:load')
if (!disabledInstrumentations.has('fetch')) {
require('../fetch')
}
if (!disabledInstrumentations.has('process')) {
require('../process')
}

const HOOK_SYMBOL = Symbol('hookExportsMap')

Expand Down
29 changes: 0 additions & 29 deletions packages/datadog-instrumentations/src/process.js

This file was deleted.

58 changes: 2 additions & 56 deletions packages/dd-trace/src/appsec/rasp.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ const { httpClientRequestStart } = require('./channels')
const { reportStackTrace } = require('./stack_trace')
const waf = require('./waf')
const { getBlockingAction, block } = require('./blocking')
const { channel } = require('dc-polyfill')

const startSetUncaughtExceptionCaptureCallback = channel('datadog:process:setUncaughtExceptionCaptureCallback:start')

class AbortError extends Error {
constructor (req, res, blockingAction) {
Expand All @@ -21,12 +18,8 @@ class AbortError extends Error {
}
}

function isAbortError (err) {
return err instanceof AbortError || err.cause instanceof AbortError
}

function handleUncaughtExceptionMonitor (err, origin) {
if (isAbortError(err)) {
if (err instanceof AbortError || err.cause instanceof AbortError) {
const { req, res, blockingAction } = err
block(req, res, web.root(req), null, blockingAction)

Expand All @@ -44,59 +37,12 @@ const RULE_TYPES = {

let config, abortOnUncaughtException

let previousCallback, userDefinedCallback

function enable (_config) {
config = _config
httpClientRequestStart.subscribe(analyzeSsrf)

process.on('uncaughtExceptionMonitor', handleUncaughtExceptionMonitor)
abortOnUncaughtException = process.execArgv?.includes('--abort-on-uncaught-exception')

if (abortOnUncaughtException) {
process.on('unhandledRejection', function (err) {
if (isAbortError(err)) {
const { req, res, blockingAction } = err
block(req, res, web.root(req), null, blockingAction)
} else {
const listeners = process.listeners('unhandledRejection')
// do not force crash if there are more listeners
if (listeners.length === 1) {
// TODO check the --unhandled-rejections flag
throw err
}
}
})

let appsecCallbackSetted = false
startSetUncaughtExceptionCaptureCallback.subscribe(({ currentCallback, newCallback, abortController }) => {
previousCallback = currentCallback
if (appsecCallbackSetted) {
userDefinedCallback = newCallback
abortController.abort()
}
})

if (process.hasUncaughtExceptionCaptureCallback()) {
process.setUncaughtExceptionCaptureCallback(null)
userDefinedCallback = previousCallback
}

const exceptionCaptureCallback = function (err) {
if (!isAbortError(err)) {
if (userDefinedCallback) {
userDefinedCallback(err)
} else {
process.setUncaughtExceptionCaptureCallback(null)

throw err // app will crash by customer app reasons
}
}
}

process.setUncaughtExceptionCaptureCallback(exceptionCaptureCallback)
appsecCallbackSetted = true
}
}

function disable () {
Expand Down Expand Up @@ -142,7 +88,7 @@ function handleResult (actions, req, res, abortController) {
if (blockingAction && abortController) {
const rootSpan = web.root(req)
// Should block only in express
if (rootSpan.context()._name === 'express.request') {
if (rootSpan.context()._name === 'express.request' && !abortOnUncaughtException) {
const abortError = new AbortError(req, res, blockingAction)
abortController.abort(abortError)

Expand Down

0 comments on commit 52a7f2c

Please sign in to comment.