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

log: wrong number of arguments (given 2, expected 0) #1321

Open
stdedos opened this issue Apr 9, 2020 · 6 comments
Open

log: wrong number of arguments (given 2, expected 0) #1321

stdedos opened this issue Apr 9, 2020 · 6 comments

Comments

@stdedos
Copy link

stdedos commented Apr 9, 2020

Disclaimer: Note that I have no idea of anything; this is pulled by pry-doc for me. And this crash is crashing everything for me beyond rescue

I am getting this stacktrace (stripped):

#<DRb::DRbRemoteError: wrong number of arguments (given 2, expected 0) (ArgumentError)>
(druby://1.1.1.1:37533) ruby/lib/ruby/gems/2.6.0/gems/yard-0.9.24/lib/yard/globals.rb:20:in `log'

and I am wondering why is this done

yard/lib/yard/globals.rb

Lines 20 to 22 in 25e77f5

def log
YARD::Logger.instance
end

or why is done like this.

@lsegal
Copy link
Owner

lsegal commented Apr 12, 2020

Please don't delete or override the issue template, all of the fields are required for a bug report.

@stdedos
Copy link
Author

stdedos commented Apr 13, 2020

I was really not given one when I filed this ticket 😕. In any case:

I don't know how to populate ## Steps to reproduce, ## Actual Output, ## Expected Output, since I am not using yard myself at all

Disclaimer: Note that I have no idea of anything; this is pulled by pry-doc for me. And this crash is crashing everything for me beyond rescue

Environment details:

  • OS: [Enter operating system / version here] Ubuntu 16.04.6 LTS
  • Ruby version (ruby -v): [Enter output of ruby -v] ruby 2.6.5p114 (2019-10-01 revision 67812) [x86_64-linux]
  • YARD version (yard -v): [Enter output of yard -v] Fetching yard-0.9.24.gem
  • Relevant software dependency/versions:
    • Fetching pry-doc-1.1.0.gem

To be honest, I don't know how will the above will help you, since I've already identified the problem at hand - but there you go.

@lsegal
Copy link
Owner

lsegal commented May 3, 2020

The issue template provides "Steps to reproduce", which is still missing from this issue:

## Steps to reproduce

This is the minimal reproduction for the issue. I've done my best to remove
all extraneous code and unique environment state on my machine before providing
these steps:

1. Run the following command: `yard doc --no-private`
2. Open browser to (URL)...
3. Click on Method List link.

## Actual Output

[Provide the full output (**please run yard with `--debug`**) or screenshots for
visual issues.]

Without knowing how to reproduce this issue, it's very difficult to resolve. Please help us help you by filling in all details of the issue template without omitting any sections, specifically, the actual steps to reproduce the issue. Thanks!

@stdedos
Copy link
Author

stdedos commented May 3, 2020

I know that the issue provides a template, and I am familiar this kind of reports, if that is what you are saying. I think this will not help you.

Steps to reproduce

This is the minimal reproduction for the issue. I've done my best to remove all extraneous code and unique environment state on my machine before providing [...]
No. This is "the" reproduction. The underlaying system cannot be further minimized, neither disclosed.

[...] these steps:

  1. Install pry-rescue, pry-doc gems
  2. Add a rescue in the form https://github.com/ConradIrwin/pry-rescue#block-form
  3. Let an exception trigger the block
  4. Investigate the error Optional
  5. Type continue, press Enter

Expected Output

Program execution continues as normal

Actual Output

Truncated stacktrace:

#<DRb::DRbRemoteError: wrong number of arguments (given 2, expected 0) (ArgumentError)>
(druby://1.1.1.1:37533) ruby/lib/ruby/gems/2.6.0/gems/yard-0.9.24/lib/yard/globals.rb:20:in `log'

I honestly do not see the point in all this, so that you would explain "why do you override the log method with your own" and "if that could be done differently / be avoided".

@lsegal
Copy link
Owner

lsegal commented May 3, 2020

Unfortunately I cannot reproduce the issue with the steps provided. continue is not a known method in my pry environment. Have you tried following the steps as you listed them?

[1] pry(main)> continue
NameError: undefined local variable or method `continue' for main:Object

I'm sure this is probably frustrating for you, but consider that it's unclear whether this is an issue with YARD, the dependencies you're using, the interplay between those deps, or, frankly, your code. Based on what I'm seeing, this seems to be related to the way pry-doc integrates with pry, which is not something I can fix in YARD itself. Unless there is a reproduction that shows YARD doing something that it was not designed to do, I would consider opening an issue against pry-doc to see how this can be resolved there, though I would again recommend finding a minimal reproduction to identify the actual culprit.

I honestly do not see the point in all this, so that you would explain "why do you override the log method with your own" and "if that could be done differently / be avoided".

Generally speaking, the point of diagnosing a bug is to resolve it in future releases. If your question is simply a loaded rhetorical way to say "please change the behavior", we need to have the root cause conversation first. If your intention is truly to ask why without expecting any fixes to the crashes you're experiencing, here you go:

Logging is global to YARD because YARD is not designed to be directly integrated into arbitrary runtimes outside of run-once style tooling. In other words, pry-doc probably shouldn't be loaded into the same runtime space as your debugged program, otherwise you will also be running into a whole host of issues around potentially monkeypatched classes (consider all of core_ext/* for example). Unfortunately it looks like you're running into one of those quirks of interleaving two incompatible runtimes.

Although the global log method can be fixed specifically, doing so would be a breaking change at this point, and so it likely will not happen. It also won't fix the other possible deeper gotchas that comes with requiring YARD directly into any large application. For YARD to work seamlessly with arbitrary codebases, it would have to be redesigned to use Refinements or other large refactors that would all involve breaking changes and would require a major version bump. At this point, many of those decisions are deeply coupled to YARD's implementation, for better or for worse.

Basically TLDR there is no actual answer to "why was it done this way" other than: because it was; a very very long time ago.

Hope that helps.

@stdedos
Copy link
Author

stdedos commented May 4, 2020

Unfortunately I cannot reproduce the issue with the steps provided. continue is not a known method in my pry environment. Have you tried following the steps as you listed them?

[1] pry(main)> continue
NameError: undefined local variable or method `continue' for main:Object

I am suspecting that you started pry, and not pry-byebug inside code (or let pry-rescue rescue).

I'm sure this is probably frustrating for you, but consider that it's unclear whether this is an issue with YARD, the dependencies you're using, the interplay between those deps, or, frankly, your code. Based on what I'm seeing, this seems to be related to the way pry-doc integrates with pry, which is not something I can fix in YARD itself. Unless there is a reproduction that shows YARD doing something that it was not designed to do, I would consider opening an issue against pry-doc to see how this can be resolved there, though I would again recommend finding a minimal reproduction to identify the actual culprit.

Due to complexities, I cannot further "conditionally catch all exceptions". The code sufficiently catches enough, but not "supposedly Syntax Errors" [1].

I honestly do not see the point in all this, so that you would explain "why do you override the log method with your own" and "if that could be done differently / be avoided".

Generally speaking, the point of diagnosing a bug is to resolve it in future releases. If your question is simply a loaded rhetorical way to say "please change the behavior", we need to have the root cause conversation first.

I guessed already that replication would be complicated for you - I was more looking for some "insight" coming from your knowledge and my stacktrace put together. Therefore, I am starting this line "rhetorically", so that maybe I can debug/fix the system itself or notify pry-doc in turn.

It is very easy, when opening a bug, that someone would be iron-clad to "please follow the template" behavior, and when they see a stacktrace that's not theirs to say "sorry, this is not pry-docs problem - the stacktrace is from Yard". Or get ignored e.g. ConradIrwin/pry-rescue#117

If your intention is truly to ask why without expecting any fixes to the crashes you're experiencing, here you go:

Logging is global to YARD because YARD is not designed to be directly integrated into arbitrary runtimes outside of run-once style tooling. In other words, pry-doc probably shouldn't be loaded into the same runtime space as your debugged program, otherwise you will also be running into a whole host of issues around potentially monkeypatched classes (consider all of core_ext/* for example). Unfortunately it looks like you're running into one of those quirks of interleaving two incompatible runtimes.

Although the global log method can be fixed specifically, doing so would be a breaking change at this point, and so it likely will not happen. It also won't fix the other possible deeper gotchas that comes with requiring YARD directly into any large application. For YARD to work seamlessly with arbitrary codebases, it would have to be redesigned to use Refinements or other large refactors that would all involve breaking changes and would require a major version bump. At this point, many of those decisions are deeply coupled to YARD's implementation, for better or for worse.

Basically TLDR there is no actual answer to "why was it done this way" other than: because it was; a very very long time ago.

I already saw log not getting fixed coming - code seemed pretty explicit about defining that in the way that it is. I was more looking to as "why": If you'd say "works for me, fix your code", and I got the same attitude from pry-doc - I know I just need to ignore these gems (as I am doing now because of no feedback). Recently, more things started working, so I started again my journey on trying to see what on earth is the root this problem.

Hope that helps.

Well, that does 😄:

Although the global log method can be fixed specifically, doing so would be a breaking change at this point, and so it likely will not happen. It also won't fix the other possible deeper gotchas that comes with requiring YARD directly into any large application. For YARD to work seamlessly with arbitrary codebases, it would have to be redesigned to use Refinements or other large refactors that would all involve breaking changes and would require a major version bump. At this point, many of those decisions are deeply coupled to YARD's implementation, for better or for worse.

You believe that this change is dangerous, too complicated, and unnecessary in Yard's mission. While essentially similar to "works for me, fix your code" (in the no-code-changes-are-coming way), it is a bit more exploratory (and polite). And it doesn't hurt that it's even more verbose, since I like to consume heterogenous information about software developing things that come my way.
In the same sense, I don't think prudent to "re-rescue" a 20-line change to add pry-rescue [^1], to guard against this.
This good enough for me to take this forward and ask pry-doc why are they pulling Yard in "main".

For my Ruby/all-around-knowledge, I'd be interested to know why was it originally re-defined differently from its original definition - but if there was no other reason / you don't remember / you won't say, that is fine for me too.

Thank you for going a bit out of your way and your insights!

Feel free to handle the issue as you see fit (given that you won't e.g. purge it). Sounds to me like a good candidate to mention somewhere (if it's not done already), that's is a bad, bad idea to load Yard in the main runtime, if someone else has been thinking that (and not just pry-doc).

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

No branches or pull requests

2 participants