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

Upgrading Sidekiq to 6.4 causes a warning #1077

Open
gingerlime opened this issue Jan 26, 2022 · 9 comments
Open

Upgrading Sidekiq to 6.4 causes a warning #1077

gingerlime opened this issue Jan 26, 2022 · 9 comments
Labels

Comments

@gingerlime
Copy link

Job arguments to Rollbar::Delay::Sidekiq do not serialize to JSON safely. This will raise an error in
Sidekiq 7.0. See https://github.com/mperham/sidekiq/wiki/Best-Practices or raise an error today
by calling `Sidekiq.strict_args!` during Sidekiq initialization.

See https://github.com/mperham/sidekiq/blob/main/Changes.md#640

@gkampjes
Copy link
Contributor

I came across this same problem.

Setting config.async_json_payload = true fixed it for me.

@isaacbowen
Copy link

isaacbowen commented Mar 7, 2022

Yep, this just bit me. We started using Sidekiq.strict_args!, and didn't immediately realize that we'd lost our Rollbar error reporting entirely.

Thanks for the tip, @gkampjes! :)

I think it's reasonable for this gem to be proactive about this, because as of right now using Sidekiq.strict_args! without async_json_payload results in Rollbar dropping everything.

cc @mperham

@waltjones
Copy link
Contributor

Using async_json_payload has better performance because the payload is only serialized once. Also if the payload needs to be truncated, this is done before adding the sidekiq queue, so it improves sidekiq ops as well.

On the next major version of the gem, async_json_payload will become the default or only behavior. The current default is preserved now for back compatibility.

@waltjones waltjones added the v4 label Mar 31, 2022
@thbar
Copy link
Contributor

thbar commented Oct 19, 2022

Thanks everyone for the report!

I think it's reasonable for this gem to be proactive about this, because as of right now using Sidekiq.strict_args! without async_json_payload results in Rollbar dropping everything.

Indeed, an error during tests or anything else to warn people would be good here.

I have a staging server with upgrades at the moment, and "everything was going smoothly", except I wasn't aware of that.

@waltjones is there a way to fix this without necessarily going async for now, to your knowledge? (my client would prefer to have some time to migrate to async) Thanks!

@choubacha
Copy link

So, we just enabled this while upgrading sidekiq but now we are getting a stack overflow during what appears to be the json deserialization process. I don't know the original offending line, however, because the trace is truncated in the logs.

@waltjones
Copy link
Contributor

@choubacha You're seeing this with the async_json_payload flag enabled?

@choubacha
Copy link

Yup. I suspect the issue maybe with the data that’s passed in but I cannot source the location of the overflow because the logs are truncated 😦

@jakub-novvak
Copy link

jakub-novvak commented Mar 22, 2023

Just FYI if you use sidekiq 7 and follow the rollbar documentation to integrate it with sidekiq it just makes it not working at all. I came across this issue using sidekiq 7 and it was failing for me with just this log:

INFO -- : [Rollbar] Scheduling item
E, [2023-03-22T11:47:33.941329 #41] ERROR -- : [Rollbar] Async handler failed, and there are no failover handlers configured. See the docs for "failover_handlers"
I, [2023-03-22T11:47:33.941399 #41]  INFO -- : [Rollbar] Details: https://rollbar.com/instance/uuid?uuid=79660ce3-755f-438c-aeb6-070cbbe19b4e (only available if report was successful)

I was not getting why it was happening but it might look as if rollbar is not working at all with sidekiq 7 since it raises an error when the payload is not serialized correctly. It's a bit confusing as the docs don't mention it at all, but after enabling async_json_payload flag it started working properly.

@waltjones
Copy link
Contributor

the docs don't mention it at all

@jakub-novvak thanks for pointing this out. I've updated the doc. https://docs.rollbar.com/docs/sidekiq-integration

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

7 participants