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(sdk-middleware-http): try json parse with catch block #1654

Merged
merged 11 commits into from
Mar 2, 2021

Conversation

markus-azer
Copy link
Member

Summary

wrap json.parse with try and catch

Description

Wrapping it so in case we receive any invalid json, instead of directly parsing it which could lead to Unexpected end of JSON input, in this case, if there is any error then it get catch and then forward body to consumer.

Copy link
Contributor

@daern91 daern91 left a comment

Choose a reason for hiding this comment

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

LGTM 👍

Copy link
Member Author

@markus-azer markus-azer left a comment

Choose a reason for hiding this comment

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

@daern91 any idea why it works with 11 ?

@codecov
Copy link

codecov bot commented Feb 3, 2021

Codecov Report

Merging #1654 (ec1ea89) into master (6d6ba53) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master    #1654   +/-   ##
=======================================
  Coverage   98.59%   98.59%           
=======================================
  Files         128      128           
  Lines        3347     3354    +7     
  Branches      774      775    +1     
=======================================
+ Hits         3300     3307    +7     
  Misses         43       43           
  Partials        4        4           
Impacted Files Coverage Δ
packages/sdk-middleware-http/src/http.js 97.67% <100.00%> (+0.20%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 6d6ba53...ec1ea89. Read the comment docs.

@@ -63,7 +63,7 @@ export const customLineItem = [
slug: 'mySlug',
taxCategory: {
typeId: 'tax-category',
id: 'fd890829-7554-4c22-a806-d6c6dcdbb0c5',
id: 'd205cc42-e399-424a-b6fc-0ef44772d6bc',
Copy link
Member Author

Choose a reason for hiding this comment

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

changed it cause i accidentally deleted it

Comment on lines +108 to +120
expect(data).toHaveLength(11)
expect(data).toContainEqual(expect.objectContaining({type: 'CartCreated'}))
expect(data).toContainEqual(expect.objectContaining({type: 'PaymentCreated'}))
expect(data).toContainEqual(expect.objectContaining({type: 'CustomerCreated'}))
expect(data).toContainEqual(expect.objectContaining({type: 'ReviewCreated'}))
expect(data).toContainEqual(expect.objectContaining({type: 'OrderCreated'}))
expect(data).toContainEqual(expect.objectContaining({type: 'CartCreated'}))
expect(data).toContainEqual(expect.objectContaining({type: 'Order'}))
expect(data).toContainEqual(expect.objectContaining({type: 'Cart'}))
expect(data).toContainEqual(expect.objectContaining({email: 'foo@bar.de'}))
expect(data).toContainEqual(expect.objectContaining({amountPlanned: { type: 'centPrecision', currencyCode: 'EUR', centAmount: 100, fractionDigits: 2}}))
expect(data).toContainEqual(expect.objectContaining({name: {de: 'deutscherListenName', en: 'englishListName'}}))
expect(data).toContainEqual(expect.objectContaining({text: 'Review text'}))
Copy link
Member Author

Choose a reason for hiding this comment

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

these checks to further be easy if new object were introduced

@markus-azer markus-azer requested a review from daern91 March 2, 2021 11:26
@markus-azer markus-azer merged commit 4957c2b into master Mar 2, 2021
@markus-azer markus-azer deleted the try-catch-json-parse branch March 2, 2021 11:55
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants