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

Record failing request arguments #305

Closed
wants to merge 2 commits into from

Conversation

bvogel
Copy link

@bvogel bvogel commented Aug 11, 2023

Currently when a POST to a service fails there is no information what parameter caused the failure, especially the device tokens of FCM delivery methods are missing. Adding the args to the exception allows to use rescue_from Noticed::ResponseUnsuccessful and have more meaningful data to react to the failure.

Specific use case: I use FCM as delivery method and sometimes during testing device tokens from a different environment will end up in the database, however the response isn't a 404 and only tells me that the request failed due to a SENDER_ID_MISMATCH. With this change I'd be able to rescue from this exception and remove the offending device_token from the database.

Additionally I took the liberty to remove 2 puts from the production code.

@bvogel
Copy link
Author

bvogel commented Aug 22, 2023

I'm really struggling without this, I keep getting

exception: {
  message: "Noticed::ResponseUnsuccessful"
  name: "Noticed::ResponseUnsuccessful"
  stacktrace: [93]
}

in our k8s logs without any insights why the request is failing

@stephaneliu
Copy link

Thanks for putting this together. Adding more insights to exception would greatly help our app as well.

@Dev-Alchemist
Copy link

I fully support this. It will undoubtedly be beneficial.

@bvogel
Copy link
Author

bvogel commented Nov 10, 2023

Hey @excid3! First of all, thank you for this great gem, it really helps us a lot. I tried giving back, however it seems this PR went un"noticed". Mind sharing your thoughts? Or is there another way to get deeper insight into fails messages?

@excid3 excid3 closed this in bdae885 Jan 15, 2024
@bvogel
Copy link
Author

bvogel commented Jan 16, 2024

Thank you @excid3!

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.

None yet

3 participants