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

always log errors with stack trace #3573

Merged
merged 1 commit into from
Mar 16, 2022
Merged

Conversation

mifi
Copy link
Contributor

@mifi mifi commented Mar 15, 2022

but don't do it for common error types that we ourselves produce

I implemented this as a middle ground. not logging stack traces for ProviderApiError and ProviderAuthError because those are part of the normal flow and not really bugs from us.

fixes #3495

but don't do it for common error types that we ourselves produce

fixes #3495
@mifi mifi requested a review from Murderlon March 15, 2022 14:42
@@ -42,7 +42,7 @@ exports.verifyToken = (req, res, next) => {
const { err, payload } = tokenService.verifyEncryptedToken(token, req.companion.options.secret)
if (err || !payload[providerName]) {
if (err) {
logger.error(err, 'token.verify.error', req.id)
logger.error(err.message, 'token.verify.error', req.id)
Copy link
Member

Choose a reason for hiding this comment

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

why not send the whole err object like in the other instances?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

because it's a very common error and causes a lot of noise, and in the current version we only log the message (inside logger.error), so I thought we'd keep the noise low on this one

@mifi mifi merged commit fc6f8f3 into main Mar 16, 2022
@mifi mifi deleted the companion-errors-with-stacktrace branch March 16, 2022 05:47
github-actions bot added a commit that referenced this pull request Mar 16, 2022
| Package              | Version | Package              | Version |
| -------------------- | ------- | -------------------- | ------- |
| @uppy/audio          |   0.3.0 | @uppy/locales        |   2.0.8 |
| @uppy/aws-s3         |   2.0.8 | @uppy/provider-views |   2.0.8 |
| @uppy/companion      |   3.4.0 | @uppy/vue            |   0.4.6 |
| @uppy/compressor     |   0.2.3 | @uppy/robodog        |   2.4.0 |
| @uppy/core           |   2.1.6 | uppy                 |   2.8.0 |
| @uppy/drop-target    |   1.1.2 |                      |         |

- @uppy/aws-s3: fix wrong events being sent to companion (Mikael Finstad / #3576)
- @uppy/compressor: ignore remote files, calculate savings correctly (Artur Paikin / #3578)
- @uppy/companion: always log errors with stack trace (Mikael Finstad / #3573)
- meta: remove incorrect s3 documentation (Mikael Finstad / #3571)
- @uppy/companion: Companion refactor (Mikael Finstad / #3542)
- website: partial ooops (Artur Paikin)
- meta: run e2e workflow on the head branch instead of the base one (Antoine du Hamel / #3561)
- website: Use Plausible instead of Google Analytics (Artur Paikin / #3567)
- @uppy/vue: enforce use of file extension within the import path (Antoine du Hamel / #3560)
- @uppy/drop-target: ignore if dropped elements aren't files (Penar Musaraj / #3563)
- @uppy/core: Abstract restriction logic in a new Restricter class (Merlijn Vos / #3532)
- @uppy/companion: Fetch all Google Drive shared drives (Robert DiMartino / #3553)
- website: add blog post 2.4-2.7 (Artur Paikin / #3557)
- meta: fix e2e (Antoine du Hamel / #3562)
- meta: fix broken link (YukeshShr / #3559)
- meta: fix support of export declaration in source files (Antoine du Hamel / #3558)
- @uppy/companion: Order Google Drive results by folder to show all folders first (Robert DiMartino / #3546)
- meta: add corsOrigins to docs (Mikael Finstad / #3554)
- @uppy/audio: refactor to ESM (Antoine du Hamel / #3470)
- @uppy/locales: compressor cleanup (Antoine du Hamel / #3531)
- meta: fix CJS interop in Vite config (Antoine du Hamel / #3543)
- @uppy/companion: upgrade node-redis-pubsub (Mikael Finstad / #3541)
- @uppy/provider-views: provider-view: fix breadcrumbs (Artur Paikin / #3535)
- meta: Update BACKLOG.md (Artur Paikin)
- @uppy/locales: Update ru_RU.js (Sobakin Sviatoslav / #3529)
- @uppy/companion: reorder reqToOptions (Antoine du Hamel / #3530)
- meta: Fix yarn caching in github actions (Mikael Finstad / #3526)
vymao pushed a commit to vymao/uppy that referenced this pull request Mar 29, 2022
| Package              | Version | Package              | Version |
| -------------------- | ------- | -------------------- | ------- |
| @uppy/audio          |   0.3.0 | @uppy/locales        |   2.0.8 |
| @uppy/aws-s3         |   2.0.8 | @uppy/provider-views |   2.0.8 |
| @uppy/companion      |   3.4.0 | @uppy/vue            |   0.4.6 |
| @uppy/compressor     |   0.2.3 | @uppy/robodog        |   2.4.0 |
| @uppy/core           |   2.1.6 | uppy                 |   2.8.0 |
| @uppy/drop-target    |   1.1.2 |                      |         |

- @uppy/aws-s3: fix wrong events being sent to companion (Mikael Finstad / transloadit#3576)
- @uppy/compressor: ignore remote files, calculate savings correctly (Artur Paikin / transloadit#3578)
- @uppy/companion: always log errors with stack trace (Mikael Finstad / transloadit#3573)
- meta: remove incorrect s3 documentation (Mikael Finstad / transloadit#3571)
- @uppy/companion: Companion refactor (Mikael Finstad / transloadit#3542)
- website: partial ooops (Artur Paikin)
- meta: run e2e workflow on the head branch instead of the base one (Antoine du Hamel / transloadit#3561)
- website: Use Plausible instead of Google Analytics (Artur Paikin / transloadit#3567)
- @uppy/vue: enforce use of file extension within the import path (Antoine du Hamel / transloadit#3560)
- @uppy/drop-target: ignore if dropped elements aren't files (Penar Musaraj / transloadit#3563)
- @uppy/core: Abstract restriction logic in a new Restricter class (Merlijn Vos / transloadit#3532)
- @uppy/companion: Fetch all Google Drive shared drives (Robert DiMartino / transloadit#3553)
- website: add blog post 2.4-2.7 (Artur Paikin / transloadit#3557)
- meta: fix e2e (Antoine du Hamel / transloadit#3562)
- meta: fix broken link (YukeshShr / transloadit#3559)
- meta: fix support of export declaration in source files (Antoine du Hamel / transloadit#3558)
- @uppy/companion: Order Google Drive results by folder to show all folders first (Robert DiMartino / transloadit#3546)
- meta: add corsOrigins to docs (Mikael Finstad / transloadit#3554)
- @uppy/audio: refactor to ESM (Antoine du Hamel / transloadit#3470)
- @uppy/locales: compressor cleanup (Antoine du Hamel / transloadit#3531)
- meta: fix CJS interop in Vite config (Antoine du Hamel / transloadit#3543)
- @uppy/companion: upgrade node-redis-pubsub (Mikael Finstad / transloadit#3541)
- @uppy/provider-views: provider-view: fix breadcrumbs (Artur Paikin / transloadit#3535)
- meta: Update BACKLOG.md (Artur Paikin)
- @uppy/locales: Update ru_RU.js (Sobakin Sviatoslav / transloadit#3529)
- @uppy/companion: reorder reqToOptions (Antoine du Hamel / transloadit#3530)
- meta: Fix yarn caching in github actions (Mikael Finstad / transloadit#3526)
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
but don't do it for common error types that we ourselves produce

fixes transloadit#3495
HeavenFox pushed a commit to docsend/uppy that referenced this pull request Jun 27, 2023
| Package              | Version | Package              | Version |
| -------------------- | ------- | -------------------- | ------- |
| @uppy/audio          |   0.3.0 | @uppy/locales        |   2.0.8 |
| @uppy/aws-s3         |   2.0.8 | @uppy/provider-views |   2.0.8 |
| @uppy/companion      |   3.4.0 | @uppy/vue            |   0.4.6 |
| @uppy/compressor     |   0.2.3 | @uppy/robodog        |   2.4.0 |
| @uppy/core           |   2.1.6 | uppy                 |   2.8.0 |
| @uppy/drop-target    |   1.1.2 |                      |         |

- @uppy/aws-s3: fix wrong events being sent to companion (Mikael Finstad / transloadit#3576)
- @uppy/compressor: ignore remote files, calculate savings correctly (Artur Paikin / transloadit#3578)
- @uppy/companion: always log errors with stack trace (Mikael Finstad / transloadit#3573)
- meta: remove incorrect s3 documentation (Mikael Finstad / transloadit#3571)
- @uppy/companion: Companion refactor (Mikael Finstad / transloadit#3542)
- website: partial ooops (Artur Paikin)
- meta: run e2e workflow on the head branch instead of the base one (Antoine du Hamel / transloadit#3561)
- website: Use Plausible instead of Google Analytics (Artur Paikin / transloadit#3567)
- @uppy/vue: enforce use of file extension within the import path (Antoine du Hamel / transloadit#3560)
- @uppy/drop-target: ignore if dropped elements aren't files (Penar Musaraj / transloadit#3563)
- @uppy/core: Abstract restriction logic in a new Restricter class (Merlijn Vos / transloadit#3532)
- @uppy/companion: Fetch all Google Drive shared drives (Robert DiMartino / transloadit#3553)
- website: add blog post 2.4-2.7 (Artur Paikin / transloadit#3557)
- meta: fix e2e (Antoine du Hamel / transloadit#3562)
- meta: fix broken link (YukeshShr / transloadit#3559)
- meta: fix support of export declaration in source files (Antoine du Hamel / transloadit#3558)
- @uppy/companion: Order Google Drive results by folder to show all folders first (Robert DiMartino / transloadit#3546)
- meta: add corsOrigins to docs (Mikael Finstad / transloadit#3554)
- @uppy/audio: refactor to ESM (Antoine du Hamel / transloadit#3470)
- @uppy/locales: compressor cleanup (Antoine du Hamel / transloadit#3531)
- meta: fix CJS interop in Vite config (Antoine du Hamel / transloadit#3543)
- @uppy/companion: upgrade node-redis-pubsub (Mikael Finstad / transloadit#3541)
- @uppy/provider-views: provider-view: fix breadcrumbs (Artur Paikin / transloadit#3535)
- meta: Update BACKLOG.md (Artur Paikin)
- @uppy/locales: Update ru_RU.js (Sobakin Sviatoslav / transloadit#3529)
- @uppy/companion: reorder reqToOptions (Antoine du Hamel / transloadit#3530)
- meta: Fix yarn caching in github actions (Mikael Finstad / transloadit#3526)
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.

Always log errors with stack traces?
2 participants