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

feat(data-exploration): errors in data table #13703

Closed
wants to merge 2 commits into from

Conversation

mariusandra
Copy link
Collaborator

Problem

The data exploration events table swallowed all errors. No longer!

Changes

Before:

2023-01-13 11 47 01

After:

2023-01-13 11 46 09

Questions

I don't think we're exposing anything secret by passing on the ClickHouse error. However I don't know. Could we leak credentials or anything else this way? 🤔

How did you test this code?

Added tests for the backend. Checked the frontend in the browser.

@Twixes
Copy link
Collaborator

Twixes commented Jan 13, 2023

Django tests red❗

@mariusandra
Copy link
Collaborator Author

No more!

@Twixes
Copy link
Collaborator

Twixes commented Jan 13, 2023

Perhaps we could use posthog.errors.code.lookup_error_code() to only pass on errors like SYNTAX_ERROR, ILLEGAL_TYPE_OF_ARGUMENT, TYPE_MISMATCH, and such (based on a whitelist)? The rest would be good old 500s.

@posthog-bot
Copy link
Contributor

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 stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

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 stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

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 stale label – otherwise this will be closed in another week.

@posthog-bot
Copy link
Contributor

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 stale label – otherwise this will be closed in another week.

@mariusandra mariusandra deleted the data-exploration-events-table-errors branch February 16, 2023 12:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants