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

Throw exceptions on early CCDB error #70

Merged
merged 2 commits into from Oct 15, 2015
Merged

Conversation

sdobbs
Copy link
Contributor

@sdobbs sdobbs commented Oct 14, 2015

Certain problems with the CCDB in DANA programs, like if an invalid variation is specified, can be difficult to diagnose. Such errors are printed to the screen with useful error messages, but are often lost because execution continues until some code segfaults later on and a long, mostly-useless stack trace is printed out.

This commit attempts to provide an early exit (and clearer representation of the error) in the case of these problems by raising exceptions in the HDDM event sources, which are called early on, if the CCDB tables can not be loaded.

@gluex
Copy link

gluex commented Oct 14, 2015

Build status for this pull request: SUCCESS

Build log: /work/halld/pull_request_test/sim-recon^early_exit_on_ccdb_problem/make_early_exit_on_ccdb_problem.log
Build report: /work/halld/pull_request_test/sim-recon^early_exit_on_ccdb_problem/report_early_exit_on_ccdb_problem.txt
Location of build: /work/halld/pull_request_test/sim-recon^early_exit_on_ccdb_problem

@markito3
Copy link
Member

Is runtime_error the standard thing we throw? Wait...do we have a standard thing to throw?

@sdobbs
Copy link
Contributor Author

sdobbs commented Oct 14, 2015

Oh yeah, I should probably be using a JException, shouldn't I?

@pmattjlab
Copy link
Contributor

Well, I think there's probably two different things you would want to do: print a huge warning/error message, and then either default to some other variation (e.g. ""), or just abort() the program. This may be better than throwing an exception, because you can just handle it right there.

@pmattjlab
Copy link
Contributor

Nevermind, this looks fine.

pmattjlab added a commit that referenced this pull request Oct 15, 2015
@pmattjlab pmattjlab merged commit d5d1a4d into master Oct 15, 2015
@sdobbs
Copy link
Contributor Author

sdobbs commented Oct 15, 2015

I won't post the response I was in the middle of writing then =)

Honestly, more calibration table calls should probably fail harder (and I prefer throwing exceptions in libraries, since they can be handled downstream), but figuring out all of the different use cases is a lot of work - this is just getting some low-hanging fruit.

@sdobbs sdobbs deleted the early_exit_on_ccdb_problem branch November 13, 2015 20:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants