@uppy/companion-client: fix log type error #4766
Conversation
| @@ -111,7 +111,7 @@ export default class Provider extends RequestClient { | |||
| const authWindow = window.open(link, '_blank') | |||
| const handleToken = (e) => { | |||
| if (e.source !== authWindow) { | |||
| this.uppy.log.warn('ignoring event from unknown source', e) | |||
| this.uppy.log(`ignoring event from unknown source ${JSON.stringify(e.data)}`, 'warning') | |||
There was a problem hiding this comment.
try/catch for stringify not needed here?
There was a problem hiding this comment.
if e.data might not be valid, we should check that rather than rely on try/catch. But maybe it's always valid?
There was a problem hiding this comment.
Yea maybe it’s better that it crashes if the data is invalid? Then we can fix the bug when we know what to fix
There was a problem hiding this comment.
That also but in this case I meant a type check if we are doubting the validity: if (e.data).
There was a problem hiding this comment.
I dont think that’s going to help at all, because most things that are falsy will not cause JSON.stringify to fail afaik. What could make it fail however is circular references or BigInts anywhere in the tree
There was a problem hiding this comment.
Btw we should really improve our uppy logger so that it can take an arbitrary number of arguments, each either objects, errors or strings. Then we don’t have to manually do these things like json stringify when logging
There was a problem hiding this comment.
i added a try/catch just to be sure, because IMO a log statement should never throw an error
|
let's merge? |
|
+1, experiencing the same issue |
| Package | Version | Package | Version | | ---------------------- | ------- | ---------------------- | ------- | | @uppy/companion-client | 3.6.1 | @uppy/store-default | 3.1.0 | | @uppy/locales | 3.5.0 | uppy | 3.20.0 | - meta: uppy CDN: Export UIPlugin and BasePlugin (Artur Paikin / #4774) - @uppy/locales: Add missing translations to de_DE (Leonhard Melzer / #4800) - @uppy/store-default: refactor to typescript (Antoine du Hamel / #4785) - meta: improve js2ts script (Antoine du Hamel / #4786) - @uppy/companion-client: fix log type error (Mikael Finstad / #4766) - @uppy/companion-client: revert breaking change (Antoine du Hamel / #4801) - @uppy/locales: use TypeScript for source files (Antoine du Hamel / #4779) - meta: migrate AWS SDK v2 to v3 in `bin/uploadcdn` (Trivikram Kamat / #4776)
fixes
this.uppy.log.warn is not a function