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

Add a debug logging. #105

Closed
wants to merge 1 commit into from
Closed

Conversation

woodhull
Copy link

Hello! We're trying to use bandwidth for the first time, thanks for your help already in Bandwidth/ruby-bandwidth-iris#74

We're now trying to send a message using the Account we provisioned via the iris gem.

We received the error message from ruby-sdk the gem: Bandwidth::MessagingException: 403 The user does not have access to this API when we tried to send a message.

We expect this might be a permission problem? However, Bandwidth Support indicates that the only way to know the root cause of this issue is to send the raw HTTP requests we're making to the API. It's not currently possible to do that using the ruby gem, so I've tried to add support for Faraday logging as a config option.

I'm not sure however that this is the right approach? The code looks like it might be generated from a specification file -- so something may need to be added to the template?

Let us know the best approach to proceed, is this something that needs to happen in APIMATIC? I'm not familiar with this tool.

@woodhull woodhull requested review from a team as code owners June 28, 2023 22:21
@ckoegel
Copy link
Contributor

ckoegel commented Jun 29, 2023

Hey @woodhull

The current situation with this SDK is kind of a doozy so I'll do my best to explain. You are correct that pretty much all of the code in this repo was previously auto-generated by APIMatic; however, we dropped out partnership with them ~2yrs ago and have been hand maintaining this and our other SDKs since. This code would be fine to go in as is, no template needed.

The biggest thing to note is that we are planning on dropping support for this and other previous versions of the SDK in the next year or two in favor of newly auto-generated SDKs using the OpenAPI Generator project. The next major version of this SDK is currently in beta, and is what you see on the feature/openapi-generator-sdk branch. As of right now, the beta version is published on rubygems as 11.0.0.pre.beta.1.1. I am currently in the process of tidying up a few small things and filling out our test coverage for this version, which we plan to release as beta.2 some time next week. After soaking in beta for some time, it should go GA with little to no changes from the beta.2 version (though I can't 100% guarantee that 😬 ). The new version of the SDK should be more user friendly, better tested, and more fully featured than the current. More relevant to this PR, it has some built in debugging and also has the ability to directly configure the faraday connection to add the exact same logging as you've done in this PR.

If you are still malleable in your integration with us, I'd strongly suggest using the beta version to avoid having to migrate some time in the future and for a better experience now.

This PR is still very much useful to those who don't want to get on the beta (which may include you), and I'll happily merge it so we can get it released on next Wednesday, the 5th. 1 small change that needs to be done to this PR is the request_debug_log parameter needs to be added to client.rb, specifically on lines 40 and 59, to allow for this to be configured upon client initialization if the user doesn't wish to create the Configuration object separately.

Thanks for your contribution and let me know which version you guys are planning on using or if you have any other questions!

For reference, how to add the debug logging in the beta version:

Bandwidth.configure do |config|
  # config.debugging = true  # built-in limited debug logging
  config.configure_faraday_connection do |conn|  # the logging you guys would like, with added auth masking
    conn.response :logger, nil, headers: true, bodies: true, log_level: :debug do |logger|
      logger.filter(/(Authorization: )(.*)/, '\1[REDACTED]')
    end
  end
end

@woodhull
Copy link
Author

Awesome. I switched out the code in our app that was using the current released version of the gem for this branch, and got things working.

This actually resolved the issue with the weird 403 we were getting, so I'm particularly thankful for that advice. 🙏

Also excited this will allow us to easily use the net-http-persistent adapter since that was going to be my next PR. Thanks for your help here.

@woodhull
Copy link
Author

Closing this PR since we plan to use the new yet-to-be released version of this gem going forward.

@woodhull woodhull closed this Jun 30, 2023
@woodhull
Copy link
Author

One piece of feedback on the beta code:

The use of the Bandwidth.configure block for configuration setup means that it the new gem may not be thread safe for situations where the customer needs to use multiple user identities in a single ruby application.

This is not the case for us at the moment but something that occurred to me looking at the API design that might be an issue for some of your customers.

I did not spend any time tracking down if there was a workaround in how credentials might be supplied to specific client instances but there might already be something there that you're already planning to support.

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

Successfully merging this pull request may close these issues.

None yet

2 participants