Skip to content

Make sure http status code is set when Faraday Middleware is used - #1368

Merged
estolfo merged 4 commits into
elastic:mainfrom
estolfo:faraday-middleware
Feb 22, 2023
Merged

Make sure http status code is set when Faraday Middleware is used#1368
estolfo merged 4 commits into
elastic:mainfrom
estolfo:faraday-middleware

Conversation

@estolfo

@estolfo estolfo commented Feb 21, 2023

Copy link
Copy Markdown
Contributor

This PR registers a Faraday Middleware on which #on_complete can be called so that a span's http status code is set regardless of what arbitrary Faraday errors is thrown.

Thanks to @ericproulx for his preliminary work on this.

Resolves #1358

@ghost

ghost commented Feb 21, 2023

Copy link
Copy Markdown

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview previewSnapshots

Expand to view the summary

Build stats

  • Start Time: 2023-02-22T10:53:49.801+0000

  • Duration: 27 min 43 sec

Test stats 🧪

Test Results
Failed 0
Passed 44500
Skipped 79
Total 44579

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run benchmark tests : Run the benchmark test.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@ghost

ghost commented Feb 21, 2023

Copy link
Copy Markdown

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (1/1) 💚
Files 99.237% (130/131) 👍
Classes 99.237% (130/131) 👍
Lines 57.1% (2469/4324) 👎 -0.185
Conditionals 100.0% (0/0) 💚

@estolfo

estolfo commented Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

@ericproulx would you be able to test this out?

@ericproulx

Copy link
Copy Markdown
Contributor

Sure, I'll let you know :)

@ericproulx

Copy link
Copy Markdown
Contributor

Sure, I'll let you know :)

  c = Faraday.new(url: 'https://httpstat.us/404') do |faraday|
    faraday.use Faraday::Response::RaiseError
    faraday.response :json, content_type: /\bjson$/
    faraday.adapter Faraday.default_adapter
  end
  c.get

Screenshot 2023-02-21 at 20 46 38

Thanks, it works

@estolfo

estolfo commented Feb 21, 2023

Copy link
Copy Markdown
Contributor Author

@ericproulx Glad to hear it, thank you for testing and helping with the solution.

@estolfo
estolfo merged commit 2573e03 into elastic:main Feb 22, 2023
estolfo added a commit that referenced this pull request Mar 2, 2023
)

* Make sure http status code is set when Faraday Middleware is used

* Define Tracing Middleware class at the time of Faraday spy installation

* Add Changelog entry

* Add code comment
jclusso added a commit to jclusso/apm-agent-ruby that referenced this pull request Apr 13, 2023
* elastic/main: (30 commits)
  docs: remove kibana endpoint (elastic#1381)
  Update status badge (elastic#1379)
  Create single status check that can be set as required (elastic#1378)
  Remove jenkins related precommit hooks (elastic#1380)
  Migrate Jenkinsfile 2 GH Actions Workflow (elastic#1366)
  Migrate update specs to updatecli (elastic#1375)
  v4.6.2
  Fixing Faraday::RackBuilder::StackLocked (elastic#1371)
  Fix jruby docker images (elastic#1367)
  Update reference to sinatra main (elastic#1373)
  Update release:update_branch task to reference branch 4.x
  Add missing docs reference
  v4.6.1
  Add security options to docker containers (elastic#1356)
  Make sure http status code is set when Faraday Middleware is used (elastic#1368)
  Use composite action for updatecli workflow (elastic#1365)
  Fix sha source in updatecli update-specs.yml (elastic#1363)
  Add update-specs updatecli workflow (elastic#1359)
  use jruby user to run docker containers (elastic#1355)
  Close the read pipe at the right moment (elastic#1351)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

FaradaySpy -> missing response.status_code when using Faraday::Response::RaiseError middleware

3 participants