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

JDBC sources: Improve error message when an invalid cursor column is selected #13912

Closed
bleonard opened this issue Jun 18, 2022 · 12 comments · Fixed by #15317
Closed

JDBC sources: Improve error message when an invalid cursor column is selected #13912

bleonard opened this issue Jun 18, 2022 · 12 comments · Fixed by #15317
Assignees
Labels

Comments

@bleonard
Copy link
Contributor

bleonard commented Jun 18, 2022

When a non-sortable column is used as a cursor, we currently produce an error message that isn't super useful (<column type> can't be used as a cursor). Ideally, this message should include the table and column names, to assist users in tracking down which stream is causing the problem.


"java.lang.IllegalArgumentException: OTHER is not supported.
 io.airbyte.workers.general.DefaultReplicationWorker$SourceException: Source process exited with non-zero exit code 1"

Something interesting on this cloud account.
More details: https://docs.google.com/spreadsheets/d/1ja8deCtFHtFVP3QrWELsMpVCMfPB1kz0QfLG1szns1I/edit#gid=408762974

@sentry-io
Copy link

sentry-io bot commented Jun 29, 2022

Sentry issue: CONNECTOR-INCIDENT-MANAGEMENT-8

@grishick
Copy link
Contributor

grishick commented Jul 1, 2022

It appears that the user was able to set up an incremental sync with an unsupported field type.

@edgao
Copy link
Contributor

edgao commented Jul 25, 2022

after #14714 this will be reported with a slightly more informative error message (OTHER cannot be used as a cursor).

Hopefully sentry is able to recognize that as the same stacktrace, but leaving a breadcrumb comment in case it doesn't.

@bleonard
Copy link
Contributor Author

after #14714 this will be reported with a slightly more informative error message (OTHER cannot be used as a cursor).

Hopefully sentry is able to recognize that as the same stacktrace, but leaving a breadcrumb comment in case it doesn't.

It would be slightly nicer if OTHER was better explained. Is it possible to do "first_name column can not be used as a cursor. Please use integers or timestamps" or something that effect?

@edgao edgao changed the title Postgres Source Crash: OTHER is not supported Improve error message when an invalid cursor column is selected Jul 26, 2022
@edgao edgao changed the title Improve error message when an invalid cursor column is selected JDBC sources: Improve error message when an invalid cursor column is selected Jul 26, 2022
@edgao
Copy link
Contributor

edgao commented Jul 26, 2022

yeah, that's fair. I updated the issue title and description.

It requires a bit of refactoring to get the table+column name down to where the exception is thrown (or maybe to catch the exception where the those names are known?) so going to leave this be for now.

@bleonard
Copy link
Contributor Author

yeah, that's fair. I updated the issue title and description.

It requires a bit of refactoring to get the table+column name down to where the exception is thrown (or maybe to catch the exception where the those names are known?) so going to leave this be for now.

Agreed, just noting a future ideal. Thanks.

@grishick
Copy link
Contributor

grishick commented Aug 4, 2022

This started happening again two days ago for another customer

@grishick
Copy link
Contributor

grishick commented Aug 4, 2022

This connection has a lot of tables, but from the error log, I cannot tell which table is failing. Would be great if the log included the name of the table, name of the field, field value, and as much information about field type and why JDBC driver cannot map it to any known type.

2022-08-03 08:33:00 source > java.lang.IllegalArgumentException: OTHER cannot be used as a cursor.
2022-08-03 08:33:00 source > 	at io.airbyte.integrations.source.postgres.PostgresSourceOperations.setStatementField(PostgresSourceOperations.java:110) ~[io.airbyte.airbyte-integrations.connectors-source-postgres-0.39.41-alpha.jar:?]
2022-08-03 08:33:00 source > 	at io.airbyte.integrations.source.postgres.PostgresSourceOperations.setStatementField(PostgresSourceOperations.java:42) ~[io.airbyte.airbyte-integrations.connectors-source-postgres-0.39.41-alpha.jar:?]
2022-08-03 08:33:00 source > 	at io.airbyte.integrations.source.jdbc.AbstractJdbcSource.lambda$queryTableIncremental$16(AbstractJdbcSource.java:283) ~[io.airbyte.airbyte-integrations.connectors-source-jdbc-0.39.41-alpha.jar:?]
2022-08-03 08:33:00 source > 	at io.airbyte.db.jdbc.StreamingJdbcDatabase.unsafeQuery(StreamingJdbcDatabase.java:62) ~[io.airbyte.airbyte-db-db-lib-0.39.41-alpha.jar:?]
2022-08-03 08:33:00 source > 	at io.airbyte.integrations.source.jdbc.AbstractJdbcSource.lambda$queryTableIncremental$17(AbstractJdbcSource.java:273) ~[io.airbyte.airbyte-integrations.connectors-source-jdbc-0.39.41-alpha.jar:?]
2022-08-03 08:33:00 source > 	at io.airbyte.commons.util.LazyAutoCloseableIterator.computeNext(LazyAutoCloseableIterator.java:37) ~[io.airbyte-airbyte-commons-0.39.41-alpha.jar:?]
2022-08-03 08:33:00 source > 	at com.google.common.collect.AbstractIterator.tryToComputeNext(AbstractIterator.java:146) ~[guava-31.0.1-jre.jar:?]
2022-08-03 08:33:00 source > 	at com.google.common.collect.AbstractIterator.hasNext(AbstractIterator.java:141) ~[guava-31.0.1-jre.jar:?]
2022-08-03 08:33:00 source > 	at com.google.common.collect.TransformedIterator.hasNext(TransformedIterator.java:46) ~[guava-31.0.1-jre.jar:?]

@grishick
Copy link
Contributor

grishick commented Aug 4, 2022

@tuliren
Copy link
Contributor

tuliren commented Aug 9, 2022

There are already two PRs related to this issue:
#14356 - prevent users from selecting invalid columns
#15317 - improve error message

@subodh1810
Copy link
Contributor

My PR has been approved but the build is failing, the remaining thing is to fix the build and get it merged

@grishick
Copy link
Contributor

grishick commented Sep 7, 2022

@subodh1810 builds should be OK now

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 a pull request may close this issue.

5 participants