Skip to content

Conversation

larouxn
Copy link
Contributor

@larouxn larouxn commented Feb 6, 2025

I believe I have located a bug in the FacebookAds#with_session method in which it passes non-keyword arguments to Session.new on line 20. Bug being that Session instantiation only takes keyword arguments. If the method is in fact not broken, please do let me know, I could be overlooking something as I've just started working with this gem recently.

def with_session(access_token, secret = nil, api_version = DEFAULT_API_VERSION)
original_session = Session.current_session
Session.current_session = Session.new(access_token, secret, api_version)
yield
Session.current_session = original_session
end

module FacebookAds
class Session
attr_accessor :access_token, :app_secret, :api_version, :server_host
def initialize(access_token: nil,
app_secret: nil,
server_host: FacebookAds.config.server_host,
api_version: FacebookAds.config.api_version)
@access_token = access_token
@app_secret = app_secret
@server_host = server_host
@api_version = api_version
end

Here's a proof of concept of the current state versus the proposed changed state.

# current broken with_session
irb(main):004* FacebookAds.with_session(access_token, app_secret) do
irb(main):005*   FacebookAds::AdAccount.get(ad_account_id, "name")
irb(main):006> end
/home/spin/.bundle/shop-server/ruby-3.3.7/gems/facebookbusiness-21.0.2/lib/facebook_ads/session.rb:16:in `initialize': wrong number of arguments (given 3, expected 0) (ArgumentError)
        from /home/spin/.bundle/shop-server/ruby-3.3.7/gems/facebookbusiness-21.0.2/lib/facebook_ads.rb:20:in `new'
        from /home/spin/.bundle/shop-server/ruby-3.3.7/gems/facebookbusiness-21.0.2/lib/facebook_ads.rb:20:in `with_session'
        
# example fixed with_session minimal local method, irb line 32
irb(main):031* def with_session(access_token, app_secret = nil, api_version = "v21")
irb(main):032*   FacebookAds::Session.current_session = FacebookAds::Session.new(access_token:, app_secret:, api_version:)
irb(main):033*   yield
irb(main):034> end
=> :with_session
irb(main):035* with_session(access_token, app_secret) do
irb(main):036*   FacebookAds::AdAccount.get(ad_account_id, "name")
irb(main):037> end
=> #<FacebookAds::AdAccount {:id=>"act_redacted"}>
irb(main):038>

@facebook-github-bot
Copy link

@stcheng has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator.

@facebook-github-bot
Copy link

@stcheng merged this pull request in 2060570.

facebook-github-bot pushed a commit to facebook/facebook-business-sdk-codegen that referenced this pull request Feb 8, 2025
Summary:
I believe I have located a bug in the `FacebookAds#with_session` method in which it passes non-keyword arguments to `Session.new` on line 20. Bug being that `Session` instantiation only takes keyword arguments. If the method is in fact not broken, please do let me know, I could be overlooking something as I've just started working with this gem recently.

https://github.com/facebook/facebook-ruby-business-sdk/blob/ef8dc9b3be544eaf310564c750524eb4f48357d2/lib/facebook_ads.rb#L18-L23

https://github.com/facebook/facebook-ruby-business-sdk/blob/ef8dc9b3be544eaf310564c750524eb4f48357d2/lib/facebook_ads/session.rb#L12-L24

Here's a proof of concept of the current state versus the proposed changed state.

```ruby
# current broken with_session
irb(main):004* FacebookAds.with_session(access_token, app_secret) do
irb(main):005*   FacebookAds::AdAccount.get(ad_account_id, "name")
irb(main):006> end
/home/spin/.bundle/shop-server/ruby-3.3.7/gems/facebookbusiness-21.0.2/lib/facebook_ads/session.rb:16:in `initialize': wrong number of arguments (given 3, expected 0) (ArgumentError)
        from /home/spin/.bundle/shop-server/ruby-3.3.7/gems/facebookbusiness-21.0.2/lib/facebook_ads.rb:20:in `new'
        from /home/spin/.bundle/shop-server/ruby-3.3.7/gems/facebookbusiness-21.0.2/lib/facebook_ads.rb:20:in `with_session'

# example fixed with_session minimal local method, irb line 32
irb(main):031* def with_session(access_token, app_secret = nil, api_version = "v21")
irb(main):032*   FacebookAds::Session.current_session = FacebookAds::Session.new(access_token:, app_secret:, api_version:)
irb(main):033*   yield
irb(main):034> end
=> :with_session
irb(main):035* with_session(access_token, app_secret) do
irb(main):036*   FacebookAds::AdAccount.get(ad_account_id, "name")
irb(main):037> end
=> #<FacebookAds::AdAccount {:id=>"act_redacted"}>
irb(main):038>
```

X-link: facebook/facebook-ruby-business-sdk#219

Reviewed By: liliarizona

Differential Revision: D69335663

Pulled By: stcheng

fbshipit-source-id: 5b06a9799d0af65b1bb3f71e3e04a0f7fb202f4e
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.

2 participants