Conversation
🧙 Wizard CIRun the Wizard CI and test your changes against wizard-workbench example apps by replying with a GitHub comment using one of the following commands: Test all apps:
Test all apps in a directory:
Test an individual app:
Show more apps
Results will be posted here when complete. |
gewenyu99
left a comment
gewenyu99
left a comment
There was a problem hiding this comment.
wild ✨
Have some things worth double checking
| from fastapi.responses import JSONResponse | ||
| from posthog import capture | ||
|
|
||
| from app.posthog_client import posthog |
There was a problem hiding this comment.
Is this new? Used to no be like this yah?
There was a problem hiding this comment.
the old code was actually off. it should use the shared posthog_client.py using the Posthog() constructor as documented
basics/fastapi/app/routers/api.py
Outdated
| new_count = safe_count + 1 | ||
|
|
||
| capture("burrito_considered", properties={"total_considerations": new_count}) | ||
| posthog.capture("burrito_considered", properties={"total_considerations": new_count}) |
There was a problem hiding this comment.
I would check with Olly, I think he was really pushing for using just the plain capture pattern. I know it's changing lots with python these days tho
There was a problem hiding this comment.
this is a required consequence of the previous change above - the functional import proxies through posthog.api_key, and removing that raises an error
this also lists as it being deprecated atm:
https://github.com/PostHog/posthog-python/blob/795ee410682d3c28ffad759c95ccdbc5dcd9fd39/posthog/__init__.py#L867
There was a problem hiding this comment.
Ahh.... that's super weird. I remember we recommended the opposite. Do everything without a client instance and use the shared client instance.
| posthog.capture( | ||
| user.email, | ||
| "user_signed_up", | ||
| distinct_id=str(user.id), |
There was a problem hiding this comment.
We should be setting context no?
There was a problem hiding this comment.
@gewenyu99 could you clarify what you mean by context here? $set/@set_once person properties or the new_context()/identify_context() pattern?
There was a problem hiding this comment.
new_context()/identify_context() pattern?
using this pattern instead of explicitly passing in distinct ID. Basically like if you imagine a route controller:
func handle_request(req, res):
{distinct_id, session_id} = parse(req.headers)
Identify_context(...):
handle() # actual logicI believe this is the preferred logic.
| const phCookieName = Object.keys(req.cookies || {}).find( | ||
| name => name.startsWith('ph_') && name.endsWith('_posthog') | ||
| ); | ||
| if (phCookieName && req.cookies[phCookieName]) { |
There was a problem hiding this comment.
Should be sent through the headers instead of a cookie tbh. Should also be parsing session ID yah?
2 participants
fixes for some 🐛 found with v2 evaluator testing over the past few days
PII leaking as distinct_id in Laravel, FastAPI, Flask
distinct_idincapture(),identify(),identify_context(),and
feature_enabled()calls. Changed tostr(user.id)everywhere.$setperson properties ortag()calls where it belongs.Docs: https://posthog.com/docs/libraries/python
Removed calls to identify() that don't exist in FastAPI, Next.js
posthog.identify(). Replaced with$set/$set_onceinside
capture()properties.identify(). Same fix.Docs:
Undocumented module-level Python config in FastAPI, Flask
posthog.api_key = .../posthog.host = ....Posthog()constructor.posthog_client.pywithposthog = Posthog(token, host=...).posthog.shutdown().Docs: https://posthog.com/docs/libraries/python
PHP operator precedence bug in captureException in Laravel
$distinctId ?? (string) Auth::user()?->id ?? 'anonymous':(string)has higherprecedence than
??, so(string) nullevaluates to"", and"" ?? 'anonymous'returns
""since empty string isn't null.Next.js server routes used hardcoded distinct_id
usernameasdistinctIdinstead of extractingthe actual
distinct_idfrom the PostHog cookie.Docs: https://posthog.com/docs/libraries/next-js
Missing error tracking in Flask, React Router 7
@app.errorhandler(Exception)withposthog.capture_exception(e).capture_exceptions: truetoposthog.init()andwrapped app in
PostHogErrorBoundary.Docs:
Rails error tracking missing Current.user support
CurrentAttributespattern.:current_userifCurrentisn't defined.Docs: https://posthog.com/docs/libraries/ruby-on-rails
Python SDK version constraint too loose in FastAPI, Flask, Python
posthog>=3.0.0toposthog>=3.7.0.capture_exceptionwas added inv3.6.4,
enable_exception_autocapturein v3.6.0.React Native identify() signature confirmed correct
$set/$set_oncenesting pattern is correct per docs.Docs: https://posthog.com/docs/libraries/react-native