Job fails when turbo_rails sends an empty string in the argument list about solid_queue HOT 12 CLOSED

Hey @ConfusedVorlon, I'll look into this one. It's very strange, as Solid Queue doesn't do anything special with job arguments. It relies on Active Job's serialize method to store them and on Active Job's execute method to deserialize and run it 🤔 The job arguments are completely opaque to Solid Queue.

In any case, I'll investigate and might ask more questions.

from solid_queue.

@ConfusedVorlon, when you run this with Sidekiq, could you log and share what the parameters look like?

from solid_queue.

Many thanks for looking at this :)

Job: Turbo::Streams::BroadcastStreamJob
Arguments: "", {"content"=>"<turbo-stream action=\"refresh\"></turbo-stream>"}

so, it does have that empty argument.

from solid_queue.

Looking at this though - it seems wrong that there is no identifier for the refresh.
It wasn't so obvious with the other content in the solid queue case.
Digging in now to see what/if I'm doing wrong...

from solid_queue.

Thank you! I suspect that's the deserialized version of the arguments, though 🤔 Because Active Job needs to distinguish between Ruby kwargs (_aj_ruby2_keywords) and symbol keys (_aj_symbol_keys), and there you just have:

{"content"=>"<turbo-stream action=\"refresh\"></turbo-stream>"}

My suspicion is that Sidekiq is doing something to the empty string before executing the job so that it doesn't fail. The thing is that having an empty string there seems wrong, because that'd be the channel you're broadcasting to 😕 I'm not sure how this can work with Sidekiq. When you're using Sidekiq, do you see the messages being broadcasted at all?

from solid_queue.

not sure what sidekiq is doing. I'm just reading args from the sidekiq web interface.
happy to dig elsewhere if you can point me in the right direction.

I figured out what the trigger is though - jump is an optional association, and in this case it is nil.

So - I'm guessing roughly: turbo_rails is calling booking.jump, finding nil and generating an identifier from that which is an empty string.

I think I can work around this with an after_update_commit that checks before broadcasting.

It feels like this is probably something where turbo_rails could just help me out and not broadcast to a nil association.

I have no idea what the correct behaviour ought to be for solid_queue...

from solid_queue.

little more info - I was confusing this broadcast with a different one.

So - I thought it was doing a job that was being done elsewhere. Obviously in fact it is a non-job.

Nonetheless, sidekiq does seem to be behaving as if it is performing the job for whatever that's worth...

web               | [ActiveJob] Enqueued Turbo::Streams::BroadcastStreamJob (Job ID: a54de5b5-2eb2-46b4-92fc-92cf60c19aee) to Sidekiq(default) with arguments: "", {:content=>"<turbo-stream action=\"refresh\"></turbo-stream>"}
worker            | 2024-04-11T15:02:28.868Z pid=73365 tid=27it class=Turbo::Streams::BroadcastStreamJob jid=467375c301c89d35ed714e79 INFO: start
worker            | 2024-04-11T15:02:28.909Z pid=73365 tid=27it class=Turbo::Streams::BroadcastStreamJob jid=467375c301c89d35ed714e79 INFO: Performing Turbo::Streams::BroadcastStreamJob (Job ID: a54de5b5-2eb2-46b4-92fc-92cf60c19aee) from Sidekiq(default) enqueued at 2024-04-11T15:02:28.867699000Z with arguments: "", {:content=>"<turbo-stream action=\"refresh\"></turbo-stream>"}
worker            | 2024-04-11T15:02:28.909Z pid=73365 tid=27it class=Turbo::Streams::BroadcastStreamJob jid=467375c301c89d35ed714e79 INFO: Performed Turbo::Streams::BroadcastStreamJob (Job ID: a54de5b5-2eb2-46b4-92fc-92cf60c19aee) from Sidekiq(default) in 0.96ms
worker            | 2024-04-11T15:02:28.910Z pid=73365 tid=27it class=Turbo::Streams::BroadcastStreamJob jid=467375c301c89d35ed714e79 elapsed=0.041 INFO: done

from solid_queue.

Aha! That's very useful.

not sure what sidekiq is doing. I'm just reading args from the sidekiq web interface.

Ahh got it! Yes, that'd be the deserialized arguments. It's ok, don't worry about that.

It feels like this is probably something where turbo_rails could just help me out and not broadcast to a nil association.

Yes, I agree! Maybe it's better to reopen the issue you opened in the turbo-rails repo and add this info 🤔

I have no idea what the correct behaviour ought to be for solid_queue...

I think the right behaviour is the one you're getting, since for Solid Queue the job and its arguments are a sort of a black box, and doesn't have any influence on why the job might be failing here (an empty string passed to the PostgreSQL cable adapter). However, that's also true for Sidekiq, and this:

Nonetheless, sidekiq does seem to be behaving as if it is performing the job for whatever that's worth...

worker            | 2024-04-11T15:02:28.909Z pid=73365 tid=27it class=Turbo::Streams::BroadcastStreamJob jid=467375c301c89d35ed714e79 INFO: Performing Turbo::Streams::BroadcastStreamJob (Job ID: a54de5b5-2eb2-46b4-92fc-92cf60c19aee) from Sidekiq(default) enqueued at 2024-04-11T15:02:28.867699000Z with arguments: "", {:content=>"<turbo-stream action=\"refresh\"></turbo-stream>"}
worker            | 2024-04-11T15:02:28.909Z pid=73365 tid=27it class=Turbo::Streams::BroadcastStreamJob jid=467375c301c89d35ed714e79 INFO: Performed Turbo::Streams::BroadcastStreamJob (Job ID: a54de5b5-2eb2-46b4-92fc-92cf60c19aee) from Sidekiq(default) in 0.96ms

is certainly strange! Is it possible the error isn't being surfaced here, and the job is somehow ending at Sidekiq's Dead set? My Sidekiq experience is very limited, as I only used it in production about 7 years ago, but I know there are multiple ways you can handle errors in Sidekiq. Is it possible the failure is being silenced somehow?

from solid_queue.

Thank you so much for your help here. It's really appreciated.

I'll definitely open a turbo_rails issue. Not exactly sure where is best to catch this or I'd send a PR.

As far as I can tell, Sidekiq really does think that it is processing the job. It isn't incrementing the failed count - it would retry by default and keep the job around. It certainly does that with other broadcast jobs - but I haven't dug deeply enough to be 100% sure.

My suspicion is that it just carries on regardless. The empty argument seems to be a postgres specific complaint, and sidekiq isn't going anywhere near that, so I assume it just doesn't care.

I do have a slight spidey-sense, where I do wonder if solid_queue should behave differently.

Digging a bit deeper, the job is enqueued here:

https://github.com/hotwired/turbo-rails/blob/6a60eb4f72d2e834f91279b0a6f95be411ae6174/app/channels/turbo/streams/broadcasts.rb#L71

  def broadcast_refresh_later_to(*streamables, request_id: Turbo.current_request_id, **opts)
    refresh_debouncer_for(*streamables, request_id: request_id).debounce do
      Turbo::Streams::BroadcastStreamJob.perform_later stream_name_from(streamables), content: turbo_stream_refresh_tag(request_id: request_id, **opts)
    end
  end

streamables is [nil], so stream_name_from(streamables) is ""

In principle - there is nothing wrong with an empty string as an argument.

Should solid queue reject this ?

It seems like some postgres wierdness might be creeping in where it shouldn't....

from solid_queue.

I should have done about three steps ago!

So - following through all the way to the actual job.

class Turbo::Streams::BroadcastStreamJob < ActiveJob::Base
  discard_on ActiveJob::DeserializationError

  def perform(stream, content:)
    binding.b
    Turbo::StreamsChannel.broadcast_stream_to(stream, content: content)
  end
end

sidekiq does indeed call this with stream == ""

from solid_queue.

I figured I'd simplify things and just create my own job.
It works fine with empty string arguments.
So - presumably the postgres error is unrelated to solid queue.

@rosa Thank you for your help with this rabbit hole!

from solid_queue.

Aha! Thanks a lot for digging into this! I agree with your findings, the error seems unrelated to Solid Queue 👍🏻
I hope you don't run into more problems! A change in turbo-rails to improve this scenario would certainly be welcome, I'm sure. Thanks again!

from solid_queue.