Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
33 changes: 32 additions & 1 deletion lib/posthog.ex
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@ defmodule PostHog do
Main API for working with PostHog
"""

require Logger

@typedoc "Name under which an instance of PostHog supervision tree is registered."
@type supervisor_name() :: atom()

Expand Down Expand Up @@ -78,7 +80,36 @@ defmodule PostHog do
properties: properties
}

PostHog.Sender.send(event, name)
case run_before_send(Map.get(config, :before_send), event) do

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Map.get(config, :before_send) is inconsistent with how all other config keys are accessed in bare_capture (e.g., config.global_properties). Since before_send always exists in the validated config (defaulting to nil), dot-access is sufficient and matches the surrounding style.

Suggested change
case run_before_send(Map.get(config, :before_send), event) do
case run_before_send(config.before_send, event) do

Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!

nil -> :ok
event -> PostHog.Sender.send(event, name)
end
end

defp run_before_send(nil, event), do: event

defp run_before_send(before_send, event) when is_function(before_send, 1) do
case before_send.(event) do
nil ->
nil

%{} = event ->
event

other ->
Logger.error(
"PostHog before_send callback returned #{inspect(other)} instead of an event map or nil; dropping event"
)

nil
end
rescue
exception ->
Logger.error(
"PostHog before_send callback raised; dropping event: #{Exception.message(exception)}"
)

nil
end
Comment on lines +91 to 113

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Error-handling paths in run_before_send are untested

run_before_send/2 has two error branches — when the callback returns a non-nil, non-map value and when the callback raises an exception — but neither is exercised by the new tests. Per the team's preference for parameterised tests, these could live alongside the existing before_send tests as additional cases (e.g., @tag test table entries, or describe-level setup with different callbacks). Without coverage, a future refactor of the rescue block or the other -> branch won't be caught by the test suite.

Context Used: Do not attempt to comment on incorrect alphabetica... (source)


@doc false
Expand Down
6 changes: 6 additions & 0 deletions lib/posthog/config.ex
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,12 @@ defmodule PostHog.Config do
default: %{},
doc: "Map of properties that should be added to all events"
],
before_send: [
type: {:or, [{:fun, 1}, nil]},
default: nil,
doc:
"Callback invoked with the fully enriched event before it is queued. Return the event to send a modified version, or nil to drop it."
],
is_server: [
type: :boolean,
default: true,
Expand Down
1 change: 1 addition & 0 deletions lib/posthog/registry.ex
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ defmodule PostHog.Registry do
enabled: false,
api_client: nil,
global_properties: %{},
before_send: nil,
test_mode: false
}
end
Expand Down
36 changes: 36 additions & 0 deletions test/posthog_test.exs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,27 @@ defmodule PostHogTest do
refute Map.has_key?(properties, :"$is_server")
end

@tag config: [
global_properties: %{source: "global"},
before_send: &__MODULE__.modify_before_send/1,
supervisor_name: PostHog
]
test "before_send can modify fully enriched events" do
PostHog.bare_capture("case tested", "distinct_id", %{secret: "remove"})

assert [%{properties: properties}] = all_captured()
assert properties[:before_send] == true
assert properties[:saw_fully_enriched_event] == true
refute Map.has_key?(properties, :secret)
end

@tag config: [before_send: &__MODULE__.drop_before_send/1, supervisor_name: PostHog]
test "before_send can drop events" do
assert :ok = PostHog.bare_capture("case tested", "distinct_id")

assert [] = all_captured()
end

@tag config: [supervisor_name: CustomPostHog]
test "simple call for custom supervisor" do
PostHog.bare_capture(CustomPostHog, "case tested", "distinct_id")
Expand Down Expand Up @@ -260,6 +281,21 @@ defmodule PostHogTest do
end
end

def modify_before_send(event) do
saw_fully_enriched_event =
event.properties[:"$lib"] == "posthog-elixir" and
is_binary(event.properties[:"$lib_version"]) and
event.properties[:"$is_server"] == true and
event.properties[:source] == "global"

event
|> put_in([:properties, :before_send], true)
|> put_in([:properties, :saw_fully_enriched_event], saw_fully_enriched_event)
|> update_in([:properties], &Map.delete(&1, :secret))
end

def drop_before_send(_event), do: nil

describe "set_event_context/2 + get_event_context/2" do
test "default scope" do
PostHog.set_event_context("$exception", %{foo: "bar"})
Expand Down
Loading