Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Remove redundant error logging #7158

Merged
merged 1 commit into from
Sep 12, 2019
Merged

Conversation

Gudahtt
Copy link
Member

@Gudahtt Gudahtt commented Sep 11, 2019

The _fetchAll function is expected to return values, so catching errors and logging them only results in an additional error at the place where _fetchAll is called. It's better instead to let the error get thrown as normal.

In this particular case _fetchAll is only called in once place. The error is still correctly caught and logged (in the _update function)

The `_fetchAll` function is expected to return values, so catching
errors and logging them only results in an additional error at the
place where `_fetchAll` is called. It's better instead to let the
error get thrown as normal.

In this particular case `_fetchAll` is only called in once place. The
error is still correctly caught and logged (in the `_update` function)
@Gudahtt Gudahtt requested review from danjm and whymarrh September 11, 2019 21:47
@metamaskbot
Copy link
Collaborator

Builds ready [4599ace]

Copy link
Contributor

@whymarrh whymarrh left a comment

Choose a reason for hiding this comment

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

Good catch, thanks

@Gudahtt Gudahtt merged commit a00493f into develop Sep 12, 2019
@whymarrh whymarrh deleted the remove-redundant-error-logging branch September 13, 2019 02:01
Gudahtt added a commit that referenced this pull request Sep 17, 2019
…evelop

* origin/develop: (31 commits)
  Performance: Delivery optimized images (#7176)
  Add `appName` message to each locale
  Remove the disk store (#7170)
  Update @hapi/subtext as per security advisory (#7172)
  Add fixes for German translations (#7168)
  Fix recipient field of approve screen (#7171)
  3box integration 2.0 (#6972)
  ci - metamaskbot - include links to dep-viz and all artifacts (#7155)
  Replace `undefined` selectedAddress with `null` (#7161)
  Add polyfill for AbortController (#7157)
  Remove redundant error logging (#7158)
  Set minimum Firefox version to v56.2 to support Waterfox (#7156)
  ci - install deps with "--har" flag to capture network activity (#7143)
  ci - create source-map-explorer build-artifacts (#7141)
  ci - build-artifacts - generate sesify-viz for inspecting deps (#7151)
  Publish GitHub release from master branch (#7136)
  fix rinkeby spelling (#7148)
  deps - move gulp-terser-js to devDeps
  test:integration - fix renamed test data file
  lint fix
  ...
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants