feat: better integration between llma and error tracking #2593
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
Additional Comments (8)
-
packages/ai/src/openai/index.ts, line 312-336 (link)logic: inconsistent error handling - still using old pattern
This error handler wasn't updated to use
sendEventWithErrorToPosthoglike the streaming paths were. This means it won't link exceptions to trace events and won't prevent double-capturing. -
packages/ai/src/openai/index.ts, line 504-529 (link)logic: inconsistent error handling - still using old pattern
Same as other non-streaming path, needs to be updated to
sendEventWithErrorToPosthog -
packages/ai/src/openai/index.ts, line 644-665 (link)logic: inconsistent error handling - still using old pattern
Same issue in embeddings error handler
-
packages/ai/src/anthropic/index.ts, line 285-304 (link)logic: inconsistent error handling - still using old pattern
This non-streaming error handler wasn't updated to use
sendEventWithErrorToPosthog -
packages/ai/src/openai/azure.ts, line 277-302 (link)logic: inconsistent error handling - still using old pattern
Azure OpenAI non-streaming error handlers also need the same update
-
packages/ai/src/openai/azure.ts, line 448-474 (link)logic: inconsistent error handling - still using old pattern
Another Azure OpenAI error handler that needs updating
-
packages/ai/src/openai/azure.ts, line 513-532 (link)logic: inconsistent error handling - still using old pattern
Another Azure OpenAI error handler that needs updating
-
packages/ai/src/openai/azure.ts, line 576-600 (link)logic: inconsistent error handling - still using old pattern
Azure embeddings error handler also needs updating
12 files reviewed, 9 comments
Edit Code Review Agent Settings | Greptile
React with 👍 or 👎 to share your feedback on this new summary format
|
Size Change: +5.19 kB (+0.1%) Total Size: 5.42 MB
ℹ️ View Unchanged
|
andrewm4894
added
llmo
andrewm4894
left a comment
andrewm4894
left a comment
There was a problem hiding this comment.
thanks! was thinking there should be better integration between LLMA and Error tracking. We added an Errors tab recently but i think this SDK approach also opens up much deeper/native linkage that will be great for users of both products.
|
@daibhin i think just a couple of CI failures maybe |
|
This PR hasn't seen activity in a week! Should it be merged, closed, or further worked on? If you want to keep it open, post a comment or remove the |
|
This PR was closed due to lack of activity. Feel free to reopen if it's still relevant. |
|
Hmm I think we want to try get this over the line, reopening |
There was a problem hiding this comment.
Additional Comments (8)
-
packages/ai/src/anthropic/index.ts, line 286-303 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogthis promise rejection handler still uses the old pattern - it sends the event but throws the original
errorinstead of the enriched one. This means the error won't be marked as previously captured and could be double-captured by autocapture. -
packages/ai/src/openai/index.ts, line 312-336 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogsame issue as anthropic - this handler doesn't enrich the error or throw the enriched version, so the error could be double-captured.
-
packages/ai/src/openai/index.ts, line 511-529 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogsame pattern issue - not enriching/throwing enriched error
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!
-
packages/ai/src/openai/index.ts, line 654-665 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogNote: 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!
-
packages/ai/src/openai/azure.ts, line 278-302 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogNote: 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!
-
packages/ai/src/openai/azure.ts, line 457-474 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogNote: 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!
-
packages/ai/src/openai/azure.ts, line 522-532 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogNote: 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!
-
packages/ai/src/openai/azure.ts, line 590-600 (link)logic: inconsistent error enrichment - should use
sendEventWithErrorToPosthogNote: 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!
12 files reviewed, 8 comments
|
@andrewm4894 thank you for taking this! |
🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…capture The mock PostHog clients in tests don't have the options property, causing tests to fail when accessing client.options.enableExceptionAutocapture. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Current State SummaryThis PR links LLM observability (LLMA) traces with error tracking exceptions, enabling bi-directional navigation between AI traces and their associated errors in PostHog. How it works
Key changes by package
Recent fixes
CI StatusUnit tests and build pass. Playwright integration test failures are pre-existing on main (ingestion lag flakiness), not related to this PR. |
Resolved conflicts in packages/ai/package.json (kept @posthog/core dependency with updated package versions from main) and regenerated pnpm-lock.yaml
|
@PostHog/team-mobile for some reason this pr seems to want a review from someone on your team - assuming a codeowners thing maybe? |
i think we should remove this yeah |
Problem
Inspired by this thread: https://posthog.slack.com/archives/C07AA937K9A/p1759356822031919
Changes
isErrorcheck given we have the errorRelease info Sub-libraries affected
Libraries affected