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

Better Error Reporting When a Trigger Handler Class Doesn't Exist #31

Closed
kacrouse opened this issue Jun 25, 2021 · 4 comments · Fixed by #33
Closed

Better Error Reporting When a Trigger Handler Class Doesn't Exist #31

kacrouse opened this issue Jun 25, 2021 · 4 comments · Fixed by #33
Labels
enhancement New feature or request
Projects
Milestone

Comments

@kacrouse
Copy link

Is your feature request related to a problem? Please describe.
When a Trigger Handler class referenced by a Trigger Configuration record doesn't exist, a NullPointerException is thrown. It's not immediately apparent that the cause is because a class is missing, and once that is determined to be the root cause, it can take some time to figure out which one is missing.

Describe the solution you'd like
Two possible solutions:

  1. A runtime check that the class exists before trying to instantiate it and an informative error log if it doesn't. This could be done by querying the ApexClass object, or just checking if the Type is not null before instantiating it.

Type handlerType = Type.forName(handlerInfo.Class_Name__c);
rflib_TriggerHandler handler = (rflib_TriggerHandler) handlerType.newInstance();

  1. A unit test that queries existing classes and cross-references them against the classes referenced in the Trigger Configuration records. Less effective as unit tests may not always be run prior to the trigger being invoked.
@j-fischer
Copy link
Owner

Great suggestion @kacrouse , thanks for submitting this ticket. I will add a fix to the next release.

Cheers

@j-fischer j-fischer added the enhancement New feature or request label Jul 2, 2021
@j-fischer j-fischer added this to To Do in RFLIB-TF via automation Jul 2, 2021
@j-fischer j-fischer added this to the RFLIB-TF 1.3 milestone Jul 2, 2021
@j-fischer j-fischer linked a pull request Jul 2, 2021 that will close this issue
@j-fischer
Copy link
Owner

@kacrouse I created a PR for you request. Please have a look and let me know if you were thinking of a different implementation. I opted for a WARN message since it is possible that the overall configuration is still valid and the record is a stale artifact that, for example, got missed during a deployment.

I am open to increase the log level to ERROR if you feel that this should be appropriate, but waiting for your feedback first. I am planning to merge this change later this weekend, but we can always patch this later if needed.

Thanks again for the report.

@kacrouse
Copy link
Author

kacrouse commented Jul 2, 2021

Thanks for the responsiveness! Reviewed the PR and it looks great!

To the log level point—I think I would lean toward error. I don't really have my own standards for log levels, but this Stack Exchange post had some reasonable guidance. error definition below.

Any error which is fatal to the operation, but not the service or application (can't open a required file, missing data, etc.). These errors will force user (administrator, or direct user) intervention. These are usually reserved (in my apps) for incorrect connection strings, missing services, etc.

I think in our situation, the error is fatal to the operation of running the specific handler, but not the Trigger Manager service.

Just my conclusion based on some quick research, ultimately I'm fine with warn or error!

@j-fischer
Copy link
Owner

I agree! I was already on the fence about the WARN before, so I changed the log level for the message to ERROR.
RFLIB-TF v1.3 should be ready by the end of the weekend.
Thanks again!

@j-fischer j-fischer moved this from To Do to In progress in RFLIB-TF Jul 2, 2021
RFLIB-TF automation moved this from In progress to Done Jul 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
Development

Successfully merging a pull request may close this issue.

2 participants