Conversation
Co-authored-by: google-labs-jules[bot] <161369871+google-labs-jules[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Code Review
This pull request introduces a new Dart-based HTTPS time server quickstart for Firebase Cloud Functions. The changes include the core function logic in bin/server.dart, dependency management in pubspec.yaml, Firebase configuration in firebase.json, a .gitignore file, and comprehensive documentation in README.md for the quickstart, along with an update to the root README.md. Feedback from the review suggests correcting a cURL command's line continuation in the quickstart's README.md, adding error handling for invalid date formats in bin/server.dart to prevent server errors, and removing unused dart_firebase_admin and google_cloud_firestore dependencies from pubspec.yaml to improve project efficiency.
| curl -H 'Content-Type: application/json' / | ||
| -d '{"format": "MMMM d yyyy, h:mm:ss a"}' / | ||
| <function url> |
There was a problem hiding this comment.
The cURL command uses / for line continuation, which is incorrect for bash (it should be \). Additionally, / is a path separator, so this command would likely fail or behave unexpectedly when executed.
| curl -H 'Content-Type: application/json' / | |
| -d '{"format": "MMMM d yyyy, h:mm:ss a"}' / | |
| <function url> | |
| curl -H 'Content-Type: application/json' \ | |
| -d '{"format": "MMMM d yyyy, h:mm:ss a"}' \ | |
| <function url> |
| final formattedDate = DateFormat(format).format(DateTime.now()); | ||
| print('Sending formatted date: $formattedDate'); | ||
| return Response.ok(formattedDate); |
There was a problem hiding this comment.
The DateFormat constructor and format method can throw an exception (such as FormatException or ArgumentError) if the provided format string is invalid. Since the format can be supplied by the user via query parameters or the request body, this call should be wrapped in a try-catch block to return a 400 Bad Request instead of allowing the function to crash with a 500 Internal Server Error.
try {
final formattedDate = DateFormat(format).format(DateTime.now());
print('Sending formatted date: $formattedDate');
return Response.ok(formattedDate);
} catch (e) {
return Response.badRequest(body: 'Invalid date format pattern.');
}| dart_firebase_admin: | ||
| git: | ||
| url: https://github.com/firebase/firebase-admin-dart | ||
| path: packages/dart_firebase_admin | ||
| ref: main | ||
| google_cloud_firestore: | ||
| git: | ||
| url: https://github.com/firebase/firebase-admin-dart | ||
| path: packages/google_cloud_firestore | ||
| ref: main |
There was a problem hiding this comment.
* callable sample * add streaming callable sample
| runs-on: ubuntu-latest | ||
| strategy: | ||
| matrix: | ||
| dart-version: | ||
| - "3.11.4" | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
|
|
||
| - name: Set up Dart ${{ matrix.dart-version }} | ||
| uses: dart-lang/setup-dart@v1.6.0 | ||
| with: | ||
| sdk: ${{ matrix.dart-version }} | ||
|
|
||
| - name: Test Dart Samples | ||
| run: | | ||
| set -e | ||
| for dir in $(find Dart -name pubspec.yaml -exec dirname {} \;); do | ||
| echo "::group::Testing $dir" | ||
| cd "$dir" | ||
| dart pub get | ||
| dart format --set-exit-if-changed . | ||
| dart analyze . | ||
| cd - > /dev/null | ||
| echo "::endgroup::" | ||
| done |
Check warning
Code scanning / CodeQL
Workflow does not contain permissions Medium test
No description provided.