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

Fix NameError in v2 push_message due to missing VERSION constant #379

Closed

Conversation

tatematsu-k
Copy link

Summary

When using Line::Bot::V2::MessagingApi::ApiClient#push_message_with_http_info, a NameError occurs due to an uninitialized constant Line::Bot::V2::VERSION.

Reproduction Steps

  1. Use the line-bot-api gem with commit ref 5bc759e0a38f4eec5715a5504463147cdffec945
  2. Execute the following script:
   # Gemfile
   gem "line-bot-api", git: "https://github.com/line/line-bot-sdk-ruby.git", ref: "5bc759e0a38f4eec5715a5504463147cdffec945"

   line_retry_key = SecureRandom.uuid
   push_message_request = {
     to: "U123456789",
     messages: [{ ... }]
   }
   client = Line::Bot::V2::MessagingApi::ApiClient.new(channel_access_token: ENV["LINE_MESSAGING_CHANNEL_TOKEN"])
   response, status_code = client.push_message_with_http_info(push_message_request:, x_line_retry_key: line_retry_key)
  1. The following error occurs:
'Class#new': uninitialized constant Line::Bot::V2::VERSION (NameError)

          @http_headers = { 'User-Agent' => "LINE-BotSDK-Ruby/#{Line::Bot::V2::VERSION}" }.merge(http_headers)
                                                                             ^^^^^^^^^
	# ... backtrace for Rails Application

app(dev)> Line::Bot::V2::VERSION
(app):17:in '<main>': uninitialized constant Line::Bot::V2::VERSION (NameError)

@CLAassistant
Copy link

CLAassistant commented Feb 5, 2025

CLA assistant check
All committers have signed the CLA.

Copy link
Contributor

@Yang-33 Yang-33 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for your interest in v2, @tatematsu-k! Also, thank you for the patch for a feature that hasn't been released yet. We're very pleased that you're interested in this.

However, the work on v2 is not yet complete, and it hasn't been published as a library on RubyGems yet. The code must change before it's released, so please stay tuned.

@@ -199,4 +199,39 @@
expect(body._next).to eq("token")
end
end

describe 'POST /v2/bot/message/push' do
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Even without this change in lib/line/bot.rb, the test passes. This means that the test cannot detect the same failure you encountered. Can you write a test that fails without the change in lib/line/bot.rb and succeeds with the patch in lib/line/bot.rb?

@@ -23,6 +23,7 @@

# V2
require 'line/bot/v2/webhook_parser'
require 'line/bot/v2/version'
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Personally, I think it may be good to write require ... where Line::Bot::V2::VERSION is used. In other words, I think it may be good to write the require ... in lib/line/bot/v2/http_client.rb. What do you think?

@Yang-33
Copy link
Contributor

Yang-33 commented Apr 10, 2025

#504 fixed this. thank you for the patch, @tatematsu-k ! Let me close this PR.

@Yang-33 Yang-33 closed this Apr 10, 2025
github-merge-queue bot pushed a commit that referenced this pull request Apr 10, 2025
If the version file is not loaded in the library code but is loaded in
the gemspec file, issues like
(#379) load errors or
missing constants can occur.

To be cautious, the gemspec avoids requiring the version file directly.
Instead, it reads the file content. This approach also includes a simple
validation to ensure the version string follows the SemVer format, so
mistakes in the version file can be detected early.

Additionally, this change fixes an actual case where the version file
was not being required properly.
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.

4 participants