Skip to content

Add test for capture_headers in context_builder#1449

Merged
estolfo merged 1 commit intoelastic:mainfrom
mheiligers-godaddy:context_builder_headers_bug
Apr 4, 2024
Merged

Add test for capture_headers in context_builder#1449
estolfo merged 1 commit intoelastic:mainfrom
mheiligers-godaddy:context_builder_headers_bug

Conversation

@mheiligers-godaddy
Copy link
Contributor

What does this pull request do?

PR#1405 introduced a bug where if capture_headers is false, the ContextBuilder will raise "undefined method `has_key?' for nil:NilClass"

Checklist

  • I have signed the Contributor License Agreement.
  • My code follows the style guidelines of this project (See .rubocop.yml)
  • I have rebased my changes on top of the latest main branch
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

- [ ] I have made corresponding changes to the documentation

Related issues

@cla-checker-service
Copy link

cla-checker-service bot commented Mar 30, 2024

💚 CLA has been signed

@mheiligers-godaddy
Copy link
Contributor Author

❌ Author of the following commits did not sign a Contributor Agreement: 4fb1308

Please, read and sign the above mentioned agreement if you want to contribute to this project

Yes, yes I have.

@mheiligers-godaddy mheiligers-godaddy force-pushed the context_builder_headers_bug branch from 4fb1308 to 098273e Compare March 30, 2024 22:35
@estolfo
Copy link
Contributor

estolfo commented Apr 2, 2024

Hi @mheiligers-godaddy thanks a lot for opening this PR! Are you sure you're signing the CLA with the same email address you use for this GitHub account?

@mheiligers-godaddy
Copy link
Contributor Author

Hi @mheiligers-godaddy thanks a lot for opening this PR! Are you sure you're signing the CLA with the same email address you use for this GitHub account?

Yes, but I got an email from @gtback yesterday afternoon saying the system had been updated (I didn't realize there was an additional step) so the check should pass now.

Separately, I see there's a failing spec on JRuby 9.2-13 with Rails 5.2, but it seems unrelated to my change, though I admit I haven't looked into it further than the stack trace in the action output.

@estolfo estolfo self-requested a review April 3, 2024 14:25
@reakaleek
Copy link
Member

run docs-build

@estolfo estolfo merged commit eca7cd0 into elastic:main Apr 4, 2024
@sjohn-godaddy
Copy link

sjohn-godaddy commented Apr 4, 2024

Could we have a release with this bug fix? that would be very welcome, as our other alternative is to downgrade to 4.3 due to dependency to other gems. @estolfo

@estolfo
Copy link
Contributor

estolfo commented Apr 8, 2024

Hi @sjohn-godaddy, sure, we can do a release soon. Thanks for your patience!

@picandocodigo
Copy link
Member

@sjohn-godaddy @mheiligers-godaddy v4.7.3 was released with this fix.

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

5 participants