Skip to content

feat: add before_send function#64

Merged
pauldambra merged 7 commits intomainfrom
feat/add-before-send
May 20, 2025
Merged

feat: add before_send function#64
pauldambra merged 7 commits intomainfrom
feat/add-before-send

Conversation

@pauldambra
Copy link
Copy Markdown
Member

and since I'm here let's add before_send to the lib...

would help with e.g. #49

Copy link
Copy Markdown
Member

@rafaeelaudibert rafaeelaudibert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

a couple comments

Comment thread lib/posthog/client.rb Outdated
Comment thread lib/posthog/client.rb Outdated
Comment thread lib/posthog/client.rb Outdated
Comment on lines +356 to +364
processed_action = @before_send.call(action)

if processed_action.nil?
logger.warn("Event #{action[:event]} was rejected in beforeSend function")
elsif processed_action.empty?
logger.warn("Event #{action[:event]} has no properties after beforeSend function, this is likely an error")
end

processed_action
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we need a begin/rescue here to avoid problems with the @before_send call failing. We can then return nil in that case

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

wasn't sure whether to use original or drop on error...

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

went to check JS SDK and that would explode back at the user 🫠

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's less bad on JS because capture is usually called on useEffects and stuff, but it's much worse in the server where we'll just crash their entire process

Comment thread lib/posthog/client.rb
# returns Boolean of whether the item was added to the queue.
def enqueue(action)
action = process_before_send(action)
return false if action.nil? || action.empty?
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

technically this is a breaking change
but if someone was relying on the fact that we would enqueue empty objects then 🤷

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

think that's all right. Is this going on the Ruby 3+ branch or the previous version?

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

well, felt mean to just put it on 3.x since it doesn't depend on the ruby version change
so we can ship in both and then if someone wants something more from it they have to update

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah, that's all right

pauldambra and others added 4 commits May 20, 2025 14:41
Co-authored-by: Rafael Audibert <32079912+rafaeelaudibert@users.noreply.github.com>
Co-authored-by: Rafael Audibert <32079912+rafaeelaudibert@users.noreply.github.com>
@pauldambra pauldambra marked this pull request as ready for review May 20, 2025 13:50
@pauldambra pauldambra merged commit 03b7730 into main May 20, 2025
7 checks passed
@pauldambra pauldambra deleted the feat/add-before-send branch May 20, 2025 14:08
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.

2 participants