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

Add new custom ExpandedError #296

Merged
merged 7 commits into from
Jul 6, 2023
Merged

Conversation

Cruikshanks
Copy link
Member

When working on Implement SROC invoice reissuing we had to deal with a situation where we wanted to throw an error that contained details of the records in the current context.

But we were faced with the issue that we knew the error would eventually find its way to our notifiers. If we include the details in the error message we break Errbit's ability to group instances of an error. But the standard Error constructor doesn't support passing in additional values.

To avoid making things complicated for ourselves we did this

const error = new Error('Charging Module reissue request failed')
error.billingBatchExternalId = billingBatchExternalId
error.invoiceExternalId = invoiceExternalId

throw error

We know both Pino our logger will include these extra properties in its output and Errbit can still do its stuff.

Then when working on Fix omfg() in HandleErroredBillingBatch we did a review of all our omfg() calls. This led us to find this in InitiateBillingBatchService

if (liveBillRunExists) {
  throw Error(`Batch already live for region ${regionId}`)
}

This is an example of an error message that will block Errbit from grouping these errors. We could fix this by doing what we did in ReissueInvoiceService. But it's probably time to add our own custom error that allows us to do it properly.

This change adds the error and applies it where applicable.

When working on [Implement SROC invoice reissuing](#256) we had to deal with a situation where we wanted to throw an error that contained details of the records in the current context.

But we were faced with the issue that we knew the error would eventually find it's way to our notifiers. If we include the details in the error message we break Errbit's ability to group instances of an error. But the standard [Error constructor](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error) doesn't support passing in additional values.

To avoid making things complicated for ourselves we did this

```javascript
const error = new Error('Charging Module reissue request failed')
error.billingBatchExternalId = billingBatchExternalId
error.invoiceExternalId = invoiceExternalId

throw error
```

We know both Pino our logger will include these extra properties in its output and Errbit can still do its stuff.

Then when working on [Fix omfg() in HandleErroredBillingBatch](#295) we did a review of all our `omfg()` calls. This led us to find this in `InitiateBillingBatchService`

```javascript
if (liveBillRunExists) {
  throw Error(`Batch already live for region ${regionId}`)
}
```

This is an example of an error message that will block Errbit from grouping these errors. We could fix this by doing what we did in `ReissueInvoiceService`. But it's probably time to add our own custom error that allows us to do it properly.

This change adds the error and applies it where applicable.
@Cruikshanks Cruikshanks added the enhancement New feature or request label Jul 6, 2023
@Cruikshanks Cruikshanks self-assigned this Jul 6, 2023
@Cruikshanks Cruikshanks changed the title Add new custom ErrorWithData Add new custom ExpandedError Jul 6, 2023
@Cruikshanks Cruikshanks marked this pull request as ready for review July 6, 2023 11:41
@Cruikshanks Cruikshanks requested review from StuAA78 and Jozzey July 6, 2023 11:41
Copy link
Contributor

@Jozzey Jozzey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍🏼

@Cruikshanks Cruikshanks merged commit c2df43b into main Jul 6, 2023
@Cruikshanks Cruikshanks deleted the add-custom-error-with-data branch July 6, 2023 15:55
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
None yet
Development

Successfully merging this pull request may close these issues.

2 participants