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

fix: Include column name in exception when ColumnSource.cast fails #6078

Merged

Conversation

rbasralian
Copy link
Contributor

No description provided.

@rbasralian rbasralian changed the title Include column name in exception when ColumnSource.cast fails fix: Include column name in exception when ColumnSource.cast fails Sep 18, 2024
Comment on lines 111 to 117
try {
// noinspection unchecked
return rawColumnSource.cast(clazz, componentType);
} catch (ClassCastException ex) {
throw new RuntimeException(
"Error retrieving ColumnSource with type " + clazz.getName() + " for column " + sourceName, ex);
}
Copy link
Member

Choose a reason for hiding this comment

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

That’s not better, or precise enough.

  1. No throwing RTEs. Throw something more specific.
  2. You should make it clearer that you’re throwing an exception because of a type mismatch. You would have failed before for a missing column.
  3. I think the existing error messaging is clearer and more specific.

Copy link
Member

Choose a reason for hiding this comment

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

A better change would just be to add an optional "name" prefix to ColumnSource.cast, so the existing exceptions could refer to "ColumnSource ".

@rcaudy rcaudy added the core Core development tasks label Sep 18, 2024
@rcaudy rcaudy added this to the 0.37.0 milestone Sep 18, 2024
@rbasralian rbasralian merged commit 45ebf06 into deephaven:main Sep 19, 2024
16 checks passed
@rbasralian rbasralian deleted the raffi_colsource_cast_err_colname branch September 19, 2024 02:21
@github-actions github-actions bot locked and limited conversation to collaborators Sep 19, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
core Core development tasks NoDocumentationNeeded NoReleaseNotesNeeded No release notes are needed.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants