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

JSON parse issue fix for Payload response #94

Merged
merged 3 commits into from
Nov 26, 2019

Conversation

prashant-shahi
Copy link
Contributor

@prashant-shahi prashant-shahi commented Nov 21, 2019

Fixes #43

  • Catch JSON parse exception
  • Upon the exception, return the original string instead for the getData function

This change is Reviewable

- Catch JSON parse exception
- Upon the exception, return the original string instead for the getData function
Copy link

@paulftw paulftw left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @paulftw and @prashant-shahi)


generated/api_pb.js, line 574 at r1 (raw file):

proto.api.Request.prototype.clearVarsMap = function() {
  this.getVarsMap().clear();
  return this;};

You aren't editing generated files, are you?

Why did this file change, when we haven't changed the generator version in package.json / yarn.lock?


src/types.ts, line 24 at r1 (raw file):

        }

        let res: any; // tslint:disable-line no-any

I think

try { return JSON.parse(jsonStr) } catch (err) { return jsonStr }

would be a lot easier to read (with formatting). and doesn't need extra var & tslint:disable

Copy link
Contributor Author

@prashant-shahi prashant-shahi left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 2 unresolved discussions (waiting on @paulftw)


generated/api_pb.js, line 574 at r1 (raw file):

Previously, paulftw (Paul Korzhyk) wrote…

You aren't editing generated files, are you?

Why did this file change, when we haven't changed the generator version in package.json / yarn.lock?

I didn't change this file. But I have seen it happen before as well when I run npm build:protos followed by ./generate_proto.sh


src/types.ts, line 24 at r1 (raw file):

Previously, paulftw (Paul Korzhyk) wrote…

I think

try { return JSON.parse(jsonStr) } catch (err) { return jsonStr }

would be a lot easier to read (with formatting). and doesn't need extra var & tslint:disable

Done.

Copy link
Contributor Author

@prashant-shahi prashant-shahi left a comment

Choose a reason for hiding this comment

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

Reviewed 2 of 4 files at r1, 2 of 2 files at r2.
Reviewable status: all files reviewed, 2 unresolved discussions (waiting on @paulftw)


generated/api_pb.js, line 574 at r1 (raw file):

Previously, prashant-shahi (Prashant Shahi) wrote…

I didn't change this file. But I have seen it happen before as well when I run npm build:protos followed by ./generate_proto.sh

Done.

@prashant-shahi prashant-shahi merged commit 76a1be8 into master Nov 26, 2019
@prashant-shahi prashant-shahi deleted the prashant/json-parse-fix branch November 26, 2019 09:30
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

Error parsing JSON from getData()
2 participants