Skip to content

Remove redundant error logging#7158

Merged
Gudahtt merged 1 commit intodevelopfrom
remove-redundant-error-logging
Sep 12, 2019
Merged

Remove redundant error logging#7158
Gudahtt merged 1 commit intodevelopfrom
remove-redundant-error-logging

Conversation

@Gudahtt
Copy link
Copy Markdown
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
Copy Markdown
Collaborator

Builds ready [4599ace]

Copy link
Copy Markdown
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
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants