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

Fix CHA CreateBillRunService result parsing #105

Merged
merged 4 commits into from
Jan 31, 2023

Conversation

Cruikshanks
Copy link
Member

https://eaflood.atlassian.net/browse/WATER-3833

Found when working on Add GlobalNotifier to the app. If we get an error response from the SROC Charging Module API we are able to parse the result without issue. This is because it will have a JSON body.

But if Got throws an exception, which it will do if it fails to get a response, for example, when the request times out then result.response won't have a body property.

This is causing the _parseResult() method to throw an error; Unexpected token u in JSON at position 0.

This change updates _parseResult() in CreateBillRunService to be able to handle both scenarios.

https://eaflood.atlassian.net/browse/WATER-3833

Found when working on [Add GlobalNotifier to the app](#100). If we get an error response from the [SROC Charging Module API](https://github.com/DEFRA/sroc-charging-module-api) we are able to parse the result without issue. This is because it will have a JSON body.

But if Got throws an exception, which it will do if it fails to get a response, for example, when the request times out then `result.response` won't have a `body` property.

This is causing the `_parseResult()` method to throw an error; `Unexpected token u in JSON at position 0`.

This change updates `_parseResult()` in `CreateBillRunService` to be able to handle both scenarios.
@Cruikshanks Cruikshanks added the bug Something isn't working label Jan 31, 2023
@Cruikshanks Cruikshanks self-assigned this Jan 31, 2023
> Remove this useless assignment to variable "response".
@Cruikshanks Cruikshanks marked this pull request as ready for review January 31, 2023 13:15
@Cruikshanks Cruikshanks requested a review from StuAA78 January 31, 2023 13:15
Copy link
Contributor

@StuAA78 StuAA78 left a comment

Choose a reason for hiding this comment

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

Looks fine, just a tiny tweak we could maybe make but otherwise I'm happy

app/services/charging-module/create-bill-run.service.js Outdated Show resolved Hide resolved
@Cruikshanks Cruikshanks requested a review from StuAA78 January 31, 2023 14:33
@Cruikshanks Cruikshanks merged commit acb4fb9 into main Jan 31, 2023
@Cruikshanks Cruikshanks deleted the fix-create-bill-run-result-parsing branch January 31, 2023 14:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants