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

poor init failure: missing config results in NullPointerException #256

Open
pdowler opened this issue Feb 3, 2021 · 3 comments
Open

poor init failure: missing config results in NullPointerException #256

pdowler opened this issue Feb 3, 2021 · 3 comments
Labels
bug Something isn't working

Comments

@pdowler
Copy link
Member

pdowler commented Feb 3, 2021

try running with no config and see how the fail looks

ringhold: prints FATAL error (this one is OK)

usually: PropertiesReader prints 3 WARN messages and then

minoc: NPE
tantar: NPE
fenwick: NPE
critwall: NPE

raven: NumberFormatException (misssing catalina.properties causes context.xml load to fail)

luskan: RuntimeException: INIT FAIL (caused by NPE), no PropertiesReader WARN

@pdowler pdowler added the bug Something isn't working label Feb 3, 2021
@at88mph
Copy link
Member

at88mph commented Sep 12, 2022

Here is a sample of Lusaka's output:

Caused by: java.lang.NullPointerException
	at org.opencadc.luskan.LuskanConfig.getConfig(LuskanConfig.java:132) ~[classes/:?]
	at org.opencadc.luskan.LuskanConfig.initConfig(LuskanConfig.java:107) ~[classes/:?]
	at org.opencadc.luskan.LuskanInitAction.doInit(LuskanInitAction.java:90) ~[classes/:?]

I think it would be better if the PropertiesReader constructor failed in two different ways to give the caller some information; a FileNotFoundException if the file does not exist, and an IOException (or IllegalArgumentException) if the file cannot be read. The callers can let those exception messages pass through then.

The NumberFormatExceptions cannot be trapped, I don't think. They are expected in the context.xml at load time by the Java Container (Tomcat).

@pdowler
Copy link
Member Author

pdowler commented Sep 14, 2022

I agree that PropertiesReader needs to throw those and services need to catch and report errors that are clear.

There is an InvalidConfigException (currently in cadc-storage-adapter) that adapter ctors can throw... maybe that can/should be moved somewhere to be more widely usable?? For storage adapters, it can happen in services (minoc) and programs (critwall and tantar)... so not cadc-inventory-server and not cadc-rest. All the way to cadc-util?

aside: the comment in the code for InvalidConfigException says it is a "Runtime exception ..." but it is currently a checked exception. It could really be a RuntimeException. I recall recently changing TransientException to RuntimeException so it could be thrown in places that could happen.

@at88mph
Copy link
Member

at88mph commented Sep 15, 2022

Great, I can move InvalidConfigException to cadc-util and see how it fits. The impact could be largish, which is why I added a full minor version increment to cadc-util. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants