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

traces inside of a try block are not flagged as error, even if error was thrown #197

Closed
8 tasks done
Tracked by #577 ...
Domiii opened this issue Aug 11, 2020 · 1 comment
Closed
8 tasks done
Tracked by #577 ...
Assignees
Labels
enhancement New feature or request instrumentation Related to how we instrument the code in `dbux-babel-plugin`

Comments

@Domiii
Copy link
Owner

Domiii commented Aug 11, 2020

Problem

  • Error thrown in try block does not get detected correctly.
  • If error was thrown, first trace of finally block will be flagged as error trace (incorrectly).

Examples

  • error4.js: error trace flagged incorrectly - should be todos (before todos.x) but is on = trace instead.
    • similar issue in error2.js -
    • using previousTraceInContext of PopImmediate does not work with try-catch cases

Solution

We are currently only resolving errors from PopImmediate in DataProvider.resolveErrorTraces. Currently, only traces that are PopImmediate and also !isTraceFunctionExit are error traces.

  • add and instrument new TraceTypes:
    • TraceType.TryBlockExit
    • TraceType.Catch
    • TraceType.Finally
  • fix up DataProvider.resolveErrorTraces
    • remove reliance on Trace.previousTrace (it seems to be inaccurate?)
      • use previousTrace = getPreviousInContext() instead?
    • remove getReturnTraceOfRealContext check (it's more complicated than that)
    • set error on previousTrace if...:
      • trace is Pop and !isTraceReturn(previousTrace) and !isTraceFunctionExit(previousTrace)

        • false-positive on try-finally, e.g. error-try-return-finally.js and error3.js
      • trace is TraceType.Catch

      • trace is TraceType.Finally and previousTrace is in try block and not TraceType.TryExit and !isTraceReturn(previousTrace) and !isTraceFunctionExit(previousTrace)

@Domiii Domiii added enhancement New feature or request instrumentation Related to how we instrument the code in `dbux-babel-plugin` labels Aug 11, 2020
@Domiii Domiii self-assigned this Aug 11, 2020
@Domiii Domiii added the bug Something isn't working label May 7, 2021
@Domiii Domiii removed the bug Something isn't working label Sep 23, 2021
@Domiii Domiii changed the title traces inside of a try block are not flagged as error, even if error was thrown traces inside of a try block are not flagged as error, even if error was thrown + more error trace issues Nov 15, 2021
@Domiii Domiii changed the title traces inside of a try block are not flagged as error, even if error was thrown + more error trace issues traces inside of a try block are not flagged as error, even if error was thrown Nov 15, 2021
@MiccWan
Copy link
Collaborator

MiccWan commented Nov 23, 2021

New algorithm works for most cases, except for nested try-finally. See __samplesInput__/nested-finally.js(false-positive in line 6 due to rule#3)

@Domiii Domiii closed this as completed Feb 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request instrumentation Related to how we instrument the code in `dbux-babel-plugin`
Projects
None yet
Development

No branches or pull requests

2 participants