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

fix(statsd) deal with nil ctx.api on 404 #2758

Closed
wants to merge 1 commit into from

Conversation

chyh1990
Copy link

@chyh1990 chyh1990 commented Aug 2, 2017

Summary

Fix statsd plugin, which crashes on 404 requests with:

2017/08/02 11:55:05 [error] 109#0: *46932 lua entry thread aborted: runtime error: /usr/local/share/lua/5.1/kong/plugins/statsd/handler.lua:63: attempt to index field 'api' (a nil value)
stack traceback:
coroutine 0:
	/usr/local/share/lua/5.1/kong/plugins/statsd/handler.lua: in function </usr/local/share/lua/5.1/kong/plugins/statsd/handler.lua:54>, context: ngx.timer, client: 10.20.1.1, server: 0.0.0.0:8000
2017/08/02 11:55:05 [error] 109#0: *46936 lua entry thread aborted: runtime error: /usr/local/share/lua/5.1/kong/plugins/statsd/handler.lua:63: attempt to index field 'api' (a nil value)

Full changelog

  • dereference message.api only if it is not nil

@p0pr0ck5
Copy link
Contributor

p0pr0ck5 commented Aug 2, 2017

I think this should follow the pattern presented in #2702

@p0pr0ck5 p0pr0ck5 added the pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. label Aug 2, 2017
@thibaultcha
Copy link
Member

@chyh1990 Any updates on your side? I do agree with @p0pr0ck5 that this should follow the behavior of the recent datadog fix.

@p0pr0ck5
Copy link
Contributor

@chyh1990 ping? ^

@cyrilgdn
Copy link

cyrilgdn commented Dec 11, 2017

Any news? Can we help on this? We have the same problem
BTW @p0pr0ck5 , I agree that all stats plugins should have the same behavior but it can be interesting to have stats on non-api logs. (I want to be alerted if there are too many of these queries for example)

@thibaultcha
Copy link
Member

@cyrilgdn The author seems to be unresponsive (and we are sorry for the delay on our side). Feel free to propose a change of your own following @p0pr0ck5's recommendation.

@thibaultcha thibaultcha added pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc... and removed pr/changes requested Changes were requested to this PR by a maintainer. Please address them and ping back once done. labels Jan 20, 2018
@CLAassistant
Copy link

CLA assistant check
Thank you for your submission, we really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@guanlan guanlan closed this Dec 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pending author feedback Waiting for the issue author to get back to a maintainer with findings, more details, etc...
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants