[GH-92] Simplify error handling for error call responses

[bdoodo]

Summary

Simplified error handling logic by replacing their try/catch blocks with a helper function stored in utils.

Ticket Link

Fixes #92

[mattermod]

Hello @bdoodo,

Thanks for your pull request! A Core Committer will review your pull request soon. For code contributions, you can learn more about the review process here.

[mickmister]

Hi @bdoodo! Welcome to the Mattermost community 🎉

Thanks for this pull request! The code is nice and flat now 👍

I have one request to apply this strategy in a few other places. Looking through the code, a similar improvement can be added to these files:

• fCreateTicket.ts
• fDisconnect.ts
• fSubscriptions.ts
• fTarget.ts

Let me know what you think about this @bdoodo

+20 -52 Nice diff, I love seeing that!

[bdoodo]

Thanks for the encouragement, I'm on it! @mickmister

[mattermod]

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

/cc @aspleenic

[jfrerich]

Thanks, @bdoodo! This is fantastic :)

[mickmister]

Thanks for adding the improvements @bdoodo! I have a request to avoid using then() in the spots we've refactored here. The main purpose of these improvements is to make the code more readable, and more specifically more "flat".

When we have indented code, it's difficult for a developer to parse the important parts when reading the code. If we treat each await'd statement as a discrete operation, with no important logic indented, then it's much easier to read.

I apologize for taking so long to get back to this. To make sure the reviewers are aware that they need to re-review the PR, please use GitHub's review request feature, by clicking the icon with arrows here:

[bdoodo]

Sorry for the long delay; my courseload has been heavier than anticipated. Moving forward in future tasks, I'll make sure to communicate timing difficulties and only take on tasks I'm very confident I'll be able to commit to for their full review cycle.

[mickmister]

LGTM! Just one remaining suggestion to add a newline after each of the tryCallResponseWithMessage operations

[mickmister]

I genuinely feel that you can ignore this request, as there has already been a decent amount back-and-forth on the PR, and I don't want to bog down you with more requests. Though here is the suggestion:

I suggest we add a newline after every tryCallResponseWithMessage operation, mainly to improve readability.

Suggested change

	await tryCallResponseWithMessage(app.createBotDMPost(dmText), 'fOauth2Complete - Unable to create bot DM post', res);
	const callResponse: AppCallResponse = newOKCallResponse();
	await tryCallResponseWithMessage(app.createBotDMPost(dmText), 'fOauth2Complete - Unable to create bot DM post', res);
	const callResponse: AppCallResponse = newOKCallResponse();

[bdoodo]

Thanks Mick, I'm enjoying learning about codebase conventions. So since the emphasis would be on making important code readable, I should also add a space above if it looks cramped, and it's more of an art than a science, right? Feels like a silly question to ask, but when the scope is on spacing, I want to make sure I'm not making a mistake haha

[mickmister]

So since the emphasis would be on making important code readable, I should also add a space above if it looks cramped

The convention is to "place newlines after blocks". If it is a block that is preceeding the call to tryCallResponseWithMessage then there should be a newline. In the case of this project, a block would be any multi-line statement, whether that be a function call with its arguments are expanded across multiple lines, or an actual block such as an if block or for loop block.

[mattermod]

This PR has been automatically labelled "stale" because it hasn't had recent activity.
A core team member will check in on the status of the PR to help with questions.
Thank you for your contribution!

[hanzei]

@bdoodo Would you please merge merge master into your PR?