-
Notifications
You must be signed in to change notification settings - Fork 18
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
catch exceptions in handler function and wait until handler completes #251
Conversation
b179657
to
646c227
Compare
@tschneidereit I have made the change as discussed. |
// In case you want to do sonme work after the response is sent | ||
// uncomment the line below and comment out the line with | ||
// `await handler(event.request, res) | ||
// event.waitUntil(handler(event.request, res)) |
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.
happy to replace this with a boolean and if...else
statement if that would be cleaner.
@@ -18,7 +18,16 @@ async function handleEvent(event: FetchEvent) { | |||
|
|||
let res = new ResponseBuilder(resolve); | |||
|
|||
await handler(event.request, res) | |||
try { | |||
// In case you want to do sonme work after the response is sent |
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.
typo
try { | ||
// In case you want to do sonme work after the response is sent | ||
// uncomment the line below and comment out the line with | ||
// `await handler(event.request, res) |
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.
extra '`' symbol here
} catch (e: any) { | ||
res.status(500) | ||
res.send(`error in handler: ${e}`) | ||
} | ||
} | ||
|
||
// Keep wizer happy during pre-init. Should go away |
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.
missing space on the line below
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.
The issue linked here is closed -- is this still needed?
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 will clean it up. Thoughts on the comment vs using a if...else
if you want to continue processing after response has been sent back?
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 don't really have a preference.
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.
leaving it as a comment, since it is a template, we can change it if we feel the need for it.
646c227
to
b847c6f
Compare
…function is complete Signed-off-by: karthik2804 <[email protected]>
Signed-off-by: karthik2804 <[email protected]>
b847c6f
to
07fb190
Compare
This PR adds a
try...catch
to the template so that any unhandled exceptions are caught and returned as an error instead of the generic error along the lines ofevent loop error - both task and job queues are empty, but expected operations did not resolveInternal error.
This PR also modifies the template so that there is an
event.waitUntill
on the handler function so that applications can do some work even after the response is sent.