Do not fail if endEvt is None #5

Merged
merged 1 commit into from May 17, 2016

Conversation

Projects
None yet
3 participants
@guillaumep
Contributor

guillaumep commented May 5, 2016

This fixes a behaviour I've seen in this module when integrating with TurboGears and CherryPy.
An exception is raise because endEvt is false. This happens at the first request received by the application (but only at the first request).
I suspect the context starts as invalid, thus endEvt is not defined, but then becomes valid during runapp().

This proposed change fixes this behaviour. It might not produce a valid trace, but will avoid the library crashing.

Do not fail if endEvt is None
This fixes a behaviour I've seen in this module when integrating with TurboGears and CherryPy.
An exception is raise because endEvt is false. This happens at the first request received by the application (but only at the first request).
I suspect the context starts as invalid, thus endEvt is not defined, but then becomes valid during runapp().

This proposed change fixes this behaviour. It might not produce a valid trace, but will avoid the library crashing.
@guillaumep

This comment has been minimized.

Show comment
Hide comment
@guillaumep

guillaumep May 5, 2016

Contributor

Congratulations on becoming OpenSource!

Contributor

guillaumep commented May 5, 2016

Congratulations on becoming OpenSource!

@MikeA-

This comment has been minimized.

Show comment
Hide comment
@MikeA-

MikeA- May 5, 2016

Contributor

Thanks a ton for this PR, and glad you were able to self-help to get out of that error scenario! This looks like a nice safe fix. I am not even worthy of the title of python newbie ( I'm an engineering director ), I think there might be an further underlying cause to fix here. It seems the context is valid but the endEvt isn't defined (if i'm not mistaken), so I'm not sure what paths lead to this scenario. I'm going to loop in a couple of engineers who deal with this type of problem much more frequently to review this as well - @Qard / @pglombardo mind taking a look?

Thanks again @guillaumep :)

Contributor

MikeA- commented May 5, 2016

Thanks a ton for this PR, and glad you were able to self-help to get out of that error scenario! This looks like a nice safe fix. I am not even worthy of the title of python newbie ( I'm an engineering director ), I think there might be an further underlying cause to fix here. It seems the context is valid but the endEvt isn't defined (if i'm not mistaken), so I'm not sure what paths lead to this scenario. I'm going to loop in a couple of engineers who deal with this type of problem much more frequently to review this as well - @Qard / @pglombardo mind taking a look?

Thanks again @guillaumep :)

@Qard

This comment has been minimized.

Show comment
Hide comment
@Qard

Qard May 12, 2016

Contributor

Hey, sorry for the delay in reviewing this. Looks like this solves the problem. It might be better to just move the add_edge call into send_end though, like this: https://gist.github.com/Qard/8b652eb1379c140d2db0d7e6a7efe8ad

Contributor

Qard commented May 12, 2016

Hey, sorry for the delay in reviewing this. Looks like this solves the problem. It might be better to just move the add_edge call into send_end though, like this: https://gist.github.com/Qard/8b652eb1379c140d2db0d7e6a7efe8ad

@MikeA- MikeA- merged commit 067c37a into tracelytics:master May 17, 2016

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment