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

Guard against calling destroy() on a missing key. #10

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

ceejbot
Copy link
Contributor

@ceejbot ceejbot commented Mar 15, 2014

I have seen crashes in prod when requests[mkey] doesn't exist. Not sure why this is happening, which you might want to investigate, but this check is an obvious guard against throwing when it's not there.

I have seen crashes in prod when requests[mkey] doesn't exist.
@sentientwaffle
Copy link
Contributor

@ceejbot: Cherry-picked over: 0fc0b9a as part of 39ad4ed...6e0215c.

Released in zag@0.0.5.

I also added a test that triggered the crash prior to your patch. The tests run against the new LevelDB backend 😄

I wasn't able to reproduce the issue with the UI, though. The crash is indicative of a different underlying bug. It occurs when a metrics key is removed twice from its MetricsChannel, which shouldn't ever happen.

Do you have steps that make the crash happen consistently? Its probably related to switching keys while graphing live metrics.

Also, what node version are you running this on? (Probably unrelated, but I'm curious.)

@ceejbot
Copy link
Contributor Author

ceejbot commented Mar 20, 2014

Running on node 0.10.25. I've only seen it three times, maybe, and I do not have any sense of how to repro -- no clues yet about when it's happened. I'll keep a lookout for it. And that's excellent news on the leveldb back end :D

@vvoxer
Copy link

vvoxer commented Jun 14, 2018

view

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants