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

es client afterFn should check the case of resp == nil #1440

Closed
YEXINGZHE54 opened this issue Mar 23, 2019 · 4 comments
Closed

es client afterFn should check the case of resp == nil #1440

YEXINGZHE54 opened this issue Mar 23, 2019 · 4 comments

Comments

@YEXINGZHE54
Copy link
Contributor

Requirement - jaeger working with ElasticSearch

Problem - jaeger not checking whether es response is nil, and then process panic and exit

Proposal - what do you suggest to solve the problem or improve the existing situation?

add nil pointer check before using es response pointer, such as :

if response.Errors {

Any open questions to address

@objectiser
Copy link
Contributor

Hi @YEXINGZHE54 , how often does this problem occur?

Do you have the logs showing the panic - just curious if there is any other information that would indicate why a nil response would be returned?

@YEXINGZHE54
Copy link
Contributor Author

YEXINGZHE54 commented Apr 7, 2019

hi, nil response would be returned in some rare case when es client interacting with ES server but got some unexpected failure. In detail: inside file gopkg.in/olivere/elastic.v5/bulk.go#191, the *BulkService.Do(ctx context.Context) (*BulkResponse, error) function will return a nil response if any error happened, and then inside file gopkg.in/olivere/elastic.v5/bulk_processor.go#531, *bulkWorker.commit(ctx context.Context) function call afterFn, without checking whether resp is nil; and then the nil resp would be passed into After callback defined inside jaeger config.go

@objectiser
Copy link
Contributor

@YEXINGZHE54 Ok thanks - would you be interested in contributing a fix for this?

@YEXINGZHE54
Copy link
Contributor Author

OK, I would like to do it.

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

No branches or pull requests

3 participants