Add crashlytics:sourcemap:upload command behind experiment#10565
Conversation
* Implement partial source map upload command * lint * move test fixture data * improve coverage * refactor * format * fix non-determinism in test
* Use git commit hash as default app version * App version fallback hierarchy
* Update source map upload command options - Remove `telemetryServerUrl` option - Change mapping file positional argument from required to optional (defaults to root dir) * Fix mapping path fallback and add test
* Handle mismatched source map file names * Add start of line match * Rename variable * Refactor for readbility
* fix unit tests * make resilient to test failures
* update sourcemap:upload to align with API * cleanup * lint * lint
* misc bug fixes * cleanup * Lint fixes * Mock auth in spec * bypass requireauth hook in unit tests --------- Co-authored-by: Anthony Barone <tonybaroneee@gmail.com>
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try using Wiz Code VS Code Extension. |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a new command crashlytics:sourcemap:upload to upload JavaScript source maps to Crashlytics for web apps, enabled under the crashlyticsWeb experiment flag. It includes the command implementation, utility functions for archiving files, and comprehensive unit tests. The code reviewer identified several critical issues, including a potential race condition and unhandled error crash in pipeAsync, a path resolution bug when using path.relative on already relative paths, a performance concern with reading entire JS files synchronously, a resource leak where temporary zip files are not deleted, and a missing retryDelay type definition in CommandOptions.
tonybaroneee
force-pushed
the
crashlytics2
branch
from
1f2051a to
0e65b63
Compare
tonybaroneee
force-pushed
the
crashlytics2
branch
from
2dbc0d4 to
3f04575
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces the crashlytics:sourcemap:upload command, allowing users to upload JavaScript source maps for web applications to Crashlytics under the crashlyticsWeb experiment flag. The implementation includes utilities for discovering source map mappings, extracting sourceMappingURL from JS files, and uploading zipped assets to Google Cloud Storage, alongside a new MurmurHash3 hashing function. The review feedback highlights a resource leak from an unclosed file descriptor in archiveFile.ts, a path resolution bug in uploadMap when options.projectRoot is customized, a potential buffer decoding issue in getLinkedSourceMapPath, and a style guide violation regarding explicit boolean comparisons.
tonybaroneee
force-pushed
the
crashlytics2
branch
from
2576f9a to
06dc36e
Compare
joehan
left a comment
joehan
left a comment
There was a problem hiding this comment.
Mostly LGTM after a few final fixes!
4 participants
Description
The
crashlytics:sourcemap:uploadcommand is a new Firebase CLI command that enables developers of web applications to upload JavaScript source maps to Firebase Crashlytics. This is essential for de-obfuscating JavaScript stack traces collected from web apps, allowing customers to view readable crash reports within the Crashlytics dashboard. This functionality is part of the larger effort to add support for web apps within Crashlytics.Scenarios Tested
Unit + manually tested.
Sample Commands