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

Don't print heimdall stack on errors #6778

Merged
merged 1 commit into from
Feb 15, 2017

Conversation

hjdivad
Copy link
Contributor

@hjdivad hjdivad commented Feb 15, 2017

Currently this is error-prone. The stack is useful for understanding error cases but it needs to be made more resilient first.

The error case is as follows

  1. build_1
    a. something in build adds to the stack and then errors, leaving the stack unbalanced.
    b. stopAndReport(build_1) fails to stop the node.
  2. build_2
    a. start(build_2) now detachs a node it thought was not active, but because of the prior stop failure, it is still active. When the new build node is added, it is a descendant of the build_1 node.
    b. something in the build adds to the stacka nd then errors, leaving the stack unbalanced.
    c. now the stack is detached, an error case.

The better solution here is for stop in 1b. to work even when unbalanced. What should be done

  1. stopping nodes autocloses the stack to the current node.
  2. analysis tools (eg rob's visualizer) check for autoclosed nodes and highlight the nearest un-autoclosed ancestor

Currently this is error-prone.  The stack is useful for understanding
error cases but it needs to be made more resilient first.

The error case is as follows

1. build_1
  a.  something in build adds to the stack and then errors, leaving the
      stack unbalanced.
  b.  stopAndReport(build_1) fails to stop the node.
2. build_2
  a.  start(build_2) now detachs a node it thought was not active, but
      because of the prior stop failure, it is still active.  When the
      new build node is added, it is a descendant of the build_1 node.
  b.  something in the build adds to the stacka nd then errors, leaving
      the stack unbalanced.
  c.  now the stack is detached, an error case.

The better solution here is for stop in 1b. to work even when
unbalanced.  What should be done

  1.  stopping nodes autocloses the stack to the current node.
  2.  analysis tools (eg rob's visualizer) check for autoclosed nodes and
      highlight the nearest un-autoclosed ancestor
@hjdivad hjdivad requested a review from rwjblue February 15, 2017 16:49
@rwjblue
Copy link
Member

rwjblue commented Feb 15, 2017

@homu r+

@homu
Copy link
Contributor

homu commented Feb 15, 2017

📌 Commit 02ddb29 has been approved by rwjblue

@homu
Copy link
Contributor

homu commented Feb 15, 2017

⚡ Test exempted - status

@homu homu merged commit 02ddb29 into master Feb 15, 2017
homu added a commit that referenced this pull request Feb 15, 2017
…-error, r=rwjblue

Don't print heimdall stack on errors

Currently this is error-prone.  The stack is useful for understanding error cases but it needs to be made more resilient first.

The error case is as follows

1. build_1
  a.  something in build adds to the stack and then errors, leaving the stack unbalanced.
  b.  stopAndReport(build_1) fails to stop the node.
2. build_2
  a.  start(build_2) now detachs a node it thought was not active, but because of the prior stop failure, it is still active.  When the new build node is added, it is a descendant of the build_1 node.
  b.  something in the build adds to the stacka nd then errors, leaving the stack unbalanced.
  c.  now the stack is detached, an error case.

The better solution here is for stop in 1b. to work even when unbalanced.  What should be done

  1.  stopping nodes autocloses the stack to the current node.
  2.  analysis tools (eg rob's visualizer) check for autoclosed nodes and highlight the nearest un-autoclosed ancestor
@hjdivad hjdivad deleted the hjdivad/dont-report-heimdall-stack-on-error branch February 15, 2017 19:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants