v1.23.1 broke SemVer for the 1.x-stable line

[ches]

Filing this issue per @steveklabnik's request:

The integration of c1ddf97 on the 1-x-stable branch violated SemVer -- as noted in the commentary on that commit, methods like Resque.before_fork and Resque.after_fork previously returned a proc (the single registered hook that was possible) when called without an argument; they now return an Array of all the registered hooks.

I was under the impression in the past that registration of multiple hooks -- long sought-after by many -- was a major feature slated for 2.0 because of the backward incompatibility implication.

The revision referenced (2427c97) in the commit message of c1ddf97 is not actually present on 1-x-stable, the "conflict fix" commit brings in the changes so there was no actual merge. That commit appears to have been rewritten by @jonhyman on #831 -- this got rather messy I'm afraid.

See also #831 and #859.

[steveklabnik]

I was under the impression in the past that registration of multiple hooks -- long sought-after by many -- was a major feature slated for 2.0 because of the backward incompatibility implication.

That's true, but as @jonhyman mentions here: #1091 (comment) it's also because of a bug in NewRelic.

I think this might just have to be a screw-up we live with; but if we can find a way to address that issue, I'm happy to fix in a 1.26.0. :/

[Jacobkg]

@steveklabnik I'm kinda stuck on what the actual use case is for either the current or previous behavior. It would really help me making a decision if I knew how people were relying on the fact that the hooks return a proc. @ches I doubt you are still following this but maybe you could weigh in.

[ches]

@Jacobkg: I'm not sure if this answers your question, but the hooks are basically the basis for Resque plugins, so I think that doc and looking at one or two available plugins will shed light on how people are using hooks. Apologies if that's much more elementary than what you're asking, let me try to presume further what you may be looking for:

The central matter here was that previously Resque only supported one registered worker hook per type (one before_fork, one after_fork, etc.), so if you used multiple plugins that relied on worker hooks, they would stomp on each other—essentially the last to register wins and nothing else gets run. That's pretty much what the motivating problem of New Relic's Resque instrumentation amounted to, in fact. (#537)

Clearly that's rather limiting, but that's the way it was, so the API was that Resque.before_fork or Resque.after_fork, when called without a block argument, worked like accessors to return the one currently registered hook of that type: they returned a single Proc. The set of changes referred to here introduced a stack/array of hooks per type so that multiple plugins could register worker hooks, but that meant that Resque.before_fork and Resque.after_fork would then return an array of all the registered hooks (Procs). Hence the breaking API change. A welcome change, but one that needed a major version bump by semver.

As for use cases (this is probably what you're really asking for, sorry 😞)… at least one of them was just to work around the multiple hooks problem 😁 Kludges like this in a Rails app initializer or in custom internal Resque plugins to clumsily chain:

__existing_resque_after_fork = Resque.after_fork

Resque.after_fork do |job|
  MyApp.do_after_fork_reconnections!
  __existing_resque_after_fork.call(job) if __existing_resque_after_fork
end

If it were limited to that it isn't that big a deal since the change actually removes the need for a hack, but it was still a change that broke existing code (and probably surfaced at deployment time since we tended to enable Resque.inline in development). I can't recall if there were other usages now to be honest, but it's possible there were some for metaprogramming in custom plugins.

Hope that helps!

[steveklabnik]

Closing this as wontfix then, it's been a long time and we haven't had a ton of reports anyway.