-
Notifications
You must be signed in to change notification settings - Fork 388
Add EventNotificationHandler example #2129
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
Conversation
| private static final StripeClient client = new StripeClient(API_KEY); | ||
| private static final StripeEventNotificationHandler handler = |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nit: private static final variables are generally done in ALL_CAPS. I think private final would make more sense here anyway
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
that's fair! though, since we're calling it from public static void main, the compiler complains Cannot make a static reference to the non-static field. I think for the purposes of the example, it's fine as is.
| } catch (StripeException e) { | ||
| // TODO Auto-generated catch block | ||
| e.printStackTrace(); | ||
| return; | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is interesting. I hadn't thought about this before, but perhaps they want to throw here so their webhook handler can return a non-200 response. Can you throw a checked exception here, or does that no longer compile?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I played with that, but then Java complains that the handler.onV1BillingMeterErrorReportTriggered(EventNotificationHandlerEndpoint::handleMeterErrors); line generates an unhandled exception. That's a little odd, since it feels like that's just a function pointer and isn't calling the function, but i'm not sure.
Given that it's just an example, I think i'm fine with either:
- leaving the example not-quite-complete and letting users figure out how they want to handle the error
- eating the error and understanding that it's not a production-ready handler.
do you have strong feelings?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That's the thing with checked exceptions--they are considered part of the signature of the function. I think before we go to GA, we should see if we can make a checked StripeException part of the signature of the Callback.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, but I'm not sure how we communicate to java that calling onV1BillingMeterErrorReportTriggered wont throw that checked exception, but calling .handle() could throw every exception that any of the registered callback throw. I'm just not sure that's a doable java pattern
But yeah, it's a good thing to dig into before GA!
Why?
The hosted docs were a little late merging, so I wanted to make sure they got linked in the changelog. Plus I added the fully-working example from the original PR to the actual code. I also took the opportunity to make sure the hosted docs matched these working examples.
What?
See Also