Fix broken download for long hosted-meeting recordings by mikewillems · Pull Request #319 · berkmancenter/frankly

mikewillems · 2026-03-04T02:11:12Z

What is in this PR?

Downloading recordings for long events either spun indefinitely or returned a user-facing error.

There were two potential causes, this PR fixes both.

Cause 1: The function was trying to do download by streaming synchronously (bless its heart).
Cause 2: Agora splits video downloads into a max size of the lesser of 2GB or 2 hours. This could leave a synchronous function hanging until the other download chunk is requested.

More technically:

downloadRecording streamed the entire ZIP file through the Cloud Function HTTP response, and the Flutter client read the full response body into memory before creating a blob URL. For long recordings the function hit its timeout mid-stream, the connection drops, and response.bodyBytes is null. Normally one would make the function async at that point, but instead the timeout was overridden with 5 minutes instead of 1 minute, which aside from being the wrong approach, still isn't long enough for long recordings, especially depending on network connection. I also don't know what kind of egress bandwidth we have on GCS.

The CORS error visible in devtools was just the signal that we didn't have a file to download.

Changes in the codebase

Replace ZIP streaming with signed GCS URLs.

The function now lists all MP4 files under {event.id}/ in GCS, generates a short-lived signed URL for each, and returns them as JSON. File bytes never pass through the function -- the whole call returns near-instantly regardless of recording length. The Flutter client renders each URL as a clickable link; the browser handles the download when the user taps it.

Commit 1 (`caea101`): switching download scheme to signed direct-from-GCS URL

firebase/functions/js/download-recordings.js

Removed ZIP streaming via archiver and the 300-second timeout override.
List MP4 files under {event.id}/, generate signed GCS URLs, return as { recordings: [{ name, url }] }.
Errors now return structured JSON instead of throwing HttpsError inside an onRequest handler (which does not propagate correctly outside onCall).

Commit 2 (`bb80712`): hygiene

firebase/functions/package.json

Removed unused archiver dependency

Commit 3 (`6785311`): client download initiation

client/lib/features/admin/presentation/views/events_tab.dart

Parse JSON response and open each signed URL via html.window.open().
Removed blob creation, anchor click, and URL revocation.
Added dart:convert import.

Commit 4 (`e7a1fe9`): hardening the download endpoint

firebase/functions/js/download-recordings.js

Shortened signed URL TTL from 24h to 15 minutes. 24h is longer than necessary and widens the window for a leaked URL to be abused; 15 minutes is long enough for a user to click the link but short enough to meaningfully limit exposure.
Added eventPathRegex validation before passing eventPath to firestore.doc(). Without it any caller with a valid auth token could supply an arbitrary Firestore path and read documents outside the events collection. The regex enforces the expected shape: community/{id}/templates/{id}/events/{id} (derived from the FirestoreDatabase.templateReference path builder and FirestoreEventService.eventsCollection).
Sort mp4Files by name before generating URLs so recordings are returned in a deterministic order regardless of GCS list ordering.

Commit 5 (`e7df3f3`): UI visual consistency, use tapped links instead of popups in case blocked

client/lib/features/admin/presentation/views/events_tab.dart

Replaced html.window.open() loop (called after an async gap, so popup blockers fire) with a stateful approach: the button press stores the returned URLs in _recordingLinks (a Map<String, List<Map<String, String>>> keyed by event ID), and _buildRecordingSection renders numbered GestureDetector links below the button once populated. Links persist on screen until the next navigation, and any subsequent button press refreshes them.
Added guards around jsonDecode (try/catch) and the recordings field (is! List check) to surface a clean user-facing error instead of a runtime cast exception if the server returns malformed JSON or an unexpected schema.
Swapped bare Text for HeightConstrainedText on the rendered links to match every other text element in the table, and removed the hardcoded fontSize: 12 (nothing in AppTextStyle uses 12; the smallest defined size is bodySmall at 14). Both changes bring the link style in line with the date link in the same row and other admin-view link patterns (Colors.blue + TextDecoration.underline).

Changes outside the codebase

None.

Testing this PR

For existing recordings (this should test it completely)

Go to any long recording over ~1h and attempt to download it.
Verify that the app doesn't freeze and that a new tab opens with the proper download.
Do the same for a recording over ~2h (maybe 3?) and verify that you get two downloads.
Start a hosted meeting with recording, keep it short.
Download recording - verify it still works for short recordings, but opens in a new tab.
Start a meeting with recording, let it run for 1h.
End the meeting and download the recording. Verify that the app continues running and that a new tab opens.
Start a meeting with recording, let it run for at least 3h. Verify that the app continues running, and TWO tabs open with two downloads (at this point Agora should split the according into two files per its docs).

Additional information

Note: This fixes downloads for the existing main-room hosted recording flow only. Per-session and breakout room download support is tracked in phases 4 and 5 of Breakout Room Recordings #9.

…ng the video parts

github-actions · 2026-03-04T02:17:51Z

Visit the preview URL for this PR (updated for commit 31be640):

https://gen-hls-bkc-7627--pr319-mw-fix-hosted-record-g1depyz4.web.app

_{(expires Fri, 20 Mar 2026 17:52:45 GMT)}

_{🔥 via Firebase Hosting GitHub Action 🌎}

_{Sign: eed668cca81618d491d024574a8f8a6003deaa8d}

…rsing and match style

Copilot

Pull request overview

Copilot reviewed 3 out of 3 changed files in this pull request and generated 3 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

client/lib/features/admin/presentation/views/events_tab.dart

firebase/functions/package.json

firebase/functions/js/download-recordings.js

Copilot

Pull request overview

Copilot reviewed 3 out of 4 changed files in this pull request and generated 4 comments.

Files not reviewed (1)

firebase/functions/package-lock.json: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

You can also share your feedback on Copilot code review. Take the survey.

Copilot · 2026-03-04T07:39:51Z

client/lib/features/admin/presentation/views/events_tab.dart

+              final idToken =
+                  await userService.firebaseAuth.currentUser?.getIdToken();
+
+              final response = await http.post(
+                Uri.parse(
+                  '${Environment.functionsUrlPrefix}/downloadRecording',
+                ),
+                headers: {'Authorization': 'Bearer $idToken'},
+                body: {'eventPath': event.fullPath},


getIdToken() can return null when there is no current user; this code would send Authorization: Bearer null, which then fails server-side. Consider guarding on idToken == null and surfacing a clear auth error before making the request.

Copilot · 2026-03-04T07:39:52Z

client/lib/features/admin/presentation/views/events_tab.dart

+              if (recordings.isEmpty) {
+                throw Exception('No recordings found for this event');
+              }



setState(() => _recordingLinks[event.id] = recordings); runs after multiple awaits. If the user navigates away while the request is in flight, this can throw "setState() called after dispose()". Consider checking if (!mounted) return; before calling setState.

Suggested change

if (!mounted) return;

firebase/functions/js/download-recordings.js

katherineqian

Going to just submit the comments I have at this stage - I completed a review of the code and it looks good to me. Still need to re-test the behavior and the UI; please re-request when ready!

katherineqian · 2026-03-04T21:33:18Z

firebase/functions/js/download-recordings.js

-            archive.on('error', (err) => {
-                console.error('Error creating zip file:', err)
-            })
+            const [files] = await bucket.getFiles({ prefix: `${event.id}/` })


So much neater!

katherineqian · 2026-03-04T21:36:54Z

client/lib/features/admin/presentation/views/events_tab.dart


  var _numToShow = 10;

+  /// Signed GCS URLs returned for each event, keyed by event ID.


Very helpful comment!

katherineqian · 2026-03-04T21:58:02Z

firebase/functions/js/download-recordings.js

+            }

+            const bucket = storage.bucket(bucketName)
            const [files] = await bucket.getFiles({ prefix: `${event.id}/` })


This is not related to the improvements you introduced, but it would have been nice to organize this bucket within folders under community/template for DX, since that's the way Firestore is currently organized. I know your plans for sensemaking also propose changes to structure here, though the proposal there was for the new path to add further levels of granularity beyond eventId.

There are definitely some concerns with backwards compatibility, though since there aren't a lot of recordings on prod currently, I think a one-time export of historical recordings for beta clients could work here if we want to reorganize. I'll follow up on Slack about this, since we should also chat about the new GCS recording path wrt breakout room recordings!

This comment is non-blocking for this PR - we can change the path in a subsequent PR.

mikewillems added 3 commits March 3, 2026 17:38

make recording download return a signed URL to GCS instead of streami…

caea101

…ng the video parts

Remove unused 'archiver' import

bb80712

download from GCS, pass auth token in header, open in new tab

6785311

mikewillems temporarily deployed to staging March 4, 2026 02:11 — with GitHub Actions Inactive

mikewillems requested review from Copilot and katherineqian March 4, 2026 02:11

Copilot started reviewing on behalf of mikewillems March 4, 2026 02:12 View session

This comment was marked as resolved.

Sign in to view

mikewillems changed the title ~~Mw/fix/hosted recordings~~ Fix broken download for long hosted-meeting recordings Mar 4, 2026

mikewillems temporarily deployed to staging March 4, 2026 04:42 — with GitHub Actions Inactive

mikewillems added 2 commits March 3, 2026 21:53

harden download endpoint w/ shorter TTL, path validation, sorting

e7a1fe9

avoid pop-ups in case blocked, use tap gesture instead, guard JSON pa…

e7df3f3

…rsing and match style

mikewillems force-pushed the mw/fix/hosted-recordings branch from 5511a52 to e7df3f3 Compare March 4, 2026 06:00

mikewillems temporarily deployed to staging March 4, 2026 06:00 — with GitHub Actions Inactive

mikewillems requested a review from Copilot March 4, 2026 06:04

Copilot started reviewing on behalf of mikewillems March 4, 2026 06:04 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

client/lib/features/admin/presentation/views/events_tab.dart Show resolved Hide resolved

firebase/functions/package.json Show resolved Hide resolved

firebase/functions/js/download-recordings.js Show resolved Hide resolved

regenerated package-lock file

f3732fd

mikewillems temporarily deployed to staging March 4, 2026 06:43 — with GitHub Actions Inactive

mikewillems requested review from Copilot and removed request for Copilot and katherineqian March 4, 2026 07:33

Copilot started reviewing on behalf of mikewillems March 4, 2026 07:34 View session

mikewillems requested a review from katherineqian March 4, 2026 07:34

Copilot started reviewing on behalf of mikewillems March 4, 2026 07:34 View session

Copilot AI reviewed Mar 4, 2026

View reviewed changes

fix: Revert deletion of .enterMeeting() call in _joinEvent().

477b364

katherineqian temporarily deployed to staging March 5, 2026 18:21 — with GitHub Actions Inactive

katherineqian temporarily deployed to staging March 5, 2026 18:27 — with GitHub Actions Inactive

mikewillems marked this pull request as draft March 6, 2026 08:26

added debug code

31be640

mikewillems temporarily deployed to staging March 6, 2026 17:46 — with GitHub Actions Inactive

katherineqian reviewed Mar 7, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix broken download for long hosted-meeting recordings#319

Fix broken download for long hosted-meeting recordings#319
mikewillems wants to merge 8 commits intostagingfrom
mw/fix/hosted-recordings

mikewillems commented Mar 4, 2026 •

edited

Loading

Uh oh!

github-actions bot commented Mar 4, 2026 •

edited

Loading

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Copilot AI Mar 4, 2026

Uh oh!

Uh oh!

Uh oh!

katherineqian left a comment

Uh oh!

katherineqian Mar 4, 2026

Uh oh!

katherineqian Mar 4, 2026

Uh oh!

katherineqian Mar 4, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants


		var _numToShow = 10;

		/// Signed GCS URLs returned for each event, keyed by event ID.

Conversation

mikewillems commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

What is in this PR?

Changes in the codebase

Commit 1 (caea101): switching download scheme to signed direct-from-GCS URL

Commit 2 (bb80712): hygiene

Commit 3 (6785311): client download initiation

Commit 4 (e7a1fe9): hardening the download endpoint

Commit 5 (e7df3f3): UI visual consistency, use tapped links instead of popups in case blocked

Changes outside the codebase

Testing this PR

For existing recordings (this should test it completely)

Additional information

Uh oh!

github-actions bot commented Mar 4, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

This comment was marked as resolved.

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

katherineqian left a comment

Choose a reason for hiding this comment

Uh oh!

katherineqian Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

katherineqian Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

katherineqian Mar 4, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

mikewillems commented Mar 4, 2026 •

edited

Loading

Commit 1 (`caea101`): switching download scheme to signed direct-from-GCS URL

Commit 2 (`bb80712`): hygiene

Commit 3 (`6785311`): client download initiation

Commit 4 (`e7a1fe9`): hardening the download endpoint

Commit 5 (`e7df3f3`): UI visual consistency, use tapped links instead of popups in case blocked

github-actions bot commented Mar 4, 2026 •

edited

Loading