Conversation
WalkthroughIntroduces a composite action ( Possibly related PRs
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/gh-pages.yaml:
- Around line 76-78: Guard the "Upload source maps to Sentry via
sentry_dart_plugin" step so it only runs when required Sentry secrets are
available and explicitly set a PR-scoped release: add an if condition checking
secrets (e.g. if: ${{ secrets.SENTRY_AUTH_TOKEN && secrets.SENTRY_ORG &&
secrets.SENTRY_PROJECT }}) on the step named "Upload source maps to Sentry via
sentry_dart_plugin", and populate the step's environment with SENTRY_AUTH_TOKEN,
SENTRY_ORG, SENTRY_PROJECT from secrets plus SENTRY_RELEASE set to a PR-scoped
value (e.g. use the pull_request number and commit SHA like pr-<number>-<sha>)
so the sentry_dart_plugin run uses the correct release and avoids failing on
forked PRs.
In @.github/workflows/image.yaml:
- Around line 51-57: The workflow uses unquoted shell variables in redirects
which triggers shellcheck SC2086; update the two run steps to quote the
destination variables so the redirects write to "$GITHUB_OUTPUT" and
"$GITHUB_ENV" (and likewise fix the other occurrences referenced around lines
144-150), and ensure the VERSION assignment and the SENTRY_RELEASE export still
reference the same symbols (VERSION, steps.version.outputs.VERSION,
SENTRY_RELEASE) but use the quoted forms for the redirect targets.
In `@Dockerfile`:
- Around line 10-11: Build is copying source maps into the runtime image via the
COPY build/web /usr/share/nginx/html step; remove .map files before they are
added to the final image by updating the Docker build flow: either add a step
that deletes map files from the build output (e.g., run a shell command to rm -f
build/web/**/*.map in a builder stage) before the COPY, or exclude *.map via
.dockerignore so COPY build/web /usr/share/nginx/html will not include source
maps. Ensure the change targets the COPY build/web /usr/share/nginx/html
operation so no .map files end up in the final nginx image.
dab246
force-pushed
the
features/upload-source-maps-web-to-sentry
branch
from
79c8832 to
713889c
Compare
There was a problem hiding this comment.
Actionable comments posted: 3
🤖 Fix all issues with AI agents
In @.github/workflows/gh-pages.yaml:
- Around line 94-96: The "Remove source maps before deploy" step currently uses
the command string "rm -rf build/web/*.js.map" which only deletes maps at the
top-level; change the run command to recursively delete all .js.map files (e.g.,
use a recursive glob like build/web/**/*.js.map or a find command) so source
maps in subdirectories are removed as well, keeping the step name "Remove source
maps before deploy" unchanged.
In @.github/workflows/image.yaml:
- Around line 63-66: The dev job uploads source maps via the step that runs
"dart run sentry_dart_plugin" but never deletes the .map files before the Docker
build, which causes source exposure; add a new step in the dev job immediately
after the "dart run sentry_dart_plugin" step (and before any Docker build/push
steps) that removes generated source maps (e.g., run: rm -f **/*.map or an
appropriate platform-safe command), or alternatively modify the Docker build
step to ensure the Dockerfile deletes .map files during image creation; target
the dev job steps around the "dart run sentry_dart_plugin" invocation and the
Docker build step to implement the removal.
- Around line 97-99: The workflow passes build-arg SENTRY_RELEASE but the
Dockerfile lacks ARG SENTRY_RELEASE so the build-arg is unused; either remove
SENTRY_RELEASE from the GitHub Actions workflow (remove the build-args line or
SENTRY_RELEASE entry) or add ARG SENTRY_RELEASE to the Dockerfile (declare ARG
SENTRY_RELEASE near other ARGs and use it in the Dockerfile as needed) so the
build-arg actually takes effect.
🧹 Nitpick comments (3)
.github/workflows/gh-pages.yaml (1)
68-71: Quote shell variables to prevent word splitting.Per shellcheck SC2086, the
$GITHUB_OUTPUTvariable should be quoted.🔧 Suggested fix
run: | # Take the line containing the version, remove 'version: ', and discard the space. VERSION=$(grep '^version:' pubspec.yaml | sed 's/version: //;s/[[:space:]]//g') - echo "VERSION=$VERSION" >> $GITHUB_OUTPUT + echo "VERSION=$VERSION" >> "$GITHUB_OUTPUT" echo "Version found: $VERSION"Dockerfile (1)
16-17: Consider separating gzip compression from container startup.Running
gzip -k -r -fon every container start adds latency. If the assets are static, pre-compress them during the Docker build instead.🔧 Suggested optimization
-CMD gzip -k -r -f /usr/share/nginx/html/ && nginx -g 'daemon off;' +# Pre-compress static assets at build time +RUN gzip -k -r -f /usr/share/nginx/html/ + +CMD ["nginx", "-g", "daemon off;"].github/workflows/image.yaml (1)
160-162: LGTM on source map removal for release.Good practice to remove source maps after uploading to Sentry but before building the Docker image. Consider using
findfor recursive removal to catch maps in subdirectories.🔧 More robust removal
# 🛡️ Remove source maps so the Docker image doesn't contain original code. - name: Remove source maps before Docker build - run: rm -rf build/web/*.js.map + run: find build/web -type f -name '*.map' -delete
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
.github/workflows/image.yaml (2)
98-108: Missingcontext: .will cause Docker build to fail.The
docker/build-push-actiondefaults to Git context (cloning the repo), which won't include the locally builtbuild/webdirectory. The Dockerfile'sCOPY build/web /usr/share/nginx/htmlwill fail becausebuild/webdoesn't exist in the Git-cloned context.🔧 Suggested fix for both dev and release jobs
- name: Build and push image uses: docker/build-push-action@v5 with: + context: . push: true platforms: "linux/amd64,linux/arm64"Apply to both the dev job (lines 98-108) and the release job (lines 195-205).
195-205: Missingcontext: .in release job as well.Same issue as the dev job - without
context: ., the Docker build will fail because the locally builtbuild/webdirectory won't be accessible.
🤖 Fix all issues with AI agents
In @.github/workflows/gh-pages.yaml:
- Around line 85-93: The step "Upload source maps to Sentry" is using secrets in
a step-level if (SENTRY_AUTH_TOKEN), which is not allowed; move the secret into
a job-level env or output and change the step if to reference that env instead.
Concretely: add SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} (and
SENTRY_ORG/SENTRY_PROJECT if needed) at the job-level env, then update the step
"Upload source maps to Sentry" to use if: ${{ env.SENTRY_AUTH_TOKEN != '' &&
github.event.pull_request.head.repo.fork == false }} so the secret is not
referenced directly in the step-level if.
🧹 Nitpick comments (2)
.github/workflows/gh-pages.yaml (1)
65-72: Add error handling for version extraction.If
pubspec.yamldoesn't contain aversion:line (unlikely but possible),VERSIONwill be empty, causing the Sentry upload to use an empty release identifier.🔧 Suggested improvement
- name: Read version from pubspec.yaml id: pubspec_version run: | # Take the line containing the version, remove 'version: ', and discard the space. VERSION=$(grep '^version:' pubspec.yaml | sed 's/version: //;s/[[:space:]]//g') + if [ -z "$VERSION" ]; then + echo "::error::Could not extract version from pubspec.yaml" + exit 1 + fi echo "VERSION=$VERSION" >> "$GITHUB_OUTPUT" echo "Version found: $VERSION".github/workflows/image.yaml (1)
51-53: Quote$GITHUB_OUTPUTto satisfy shellcheck.Line 53 uses unquoted
$GITHUB_OUTPUTwhile line 57 correctly quotes it. This inconsistency triggers shellcheck SC2086.🔧 Suggested fix
- name: Read version from pubspec.yaml id: version run: | VERSION=$(grep '^version:' pubspec.yaml | sed 's/version: //') - echo "VERSION=$VERSION" >> $GITHUB_OUTPUT + echo "VERSION=$VERSION" >> "$GITHUB_OUTPUT"
chibenwa
left a comment
chibenwa
left a comment
There was a problem hiding this comment.
Sometimes simpler is better
What's the issue with shipping source map directly in the image?
As such we make it more complex for community (and for us for on prem deployment!) to set up sentry. Also local debugging without source maps is harder.
I want @Crash-- opinion on this
|
A few things to note:
I'm not a Docker expert, so changing the Dockerfile is quite complicated. If you have any suggestions for this, please comment on this PR. |
|
Have you looked at #4276 ? |
I've looked at it, but what you've done is just flagging Flutter to create the |
It published source map, Sentry be able to find the public one when have the error. I merged it and built v0.24.1, please try to deploy to canary to test |
How can I verify it on the Sentry dashboard? |
|
Okay, we can use a simpler method: publicizing the folder. So we will publicize the
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/gh-pages.yaml (1)
22-57: Limit SENTRY secrets scope to the build step.
Job-level env exposes secrets to all steps (including third‑party actions). Scoping them to the Build Flutter Web step reduces exposure.🔒 Suggested change
- env: - SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} - SENTRY_ORG: ${{ secrets.SENTRY_ORG }} - SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }} ... - name: Build Flutter Web uses: ./.github/actions/build-flutter-web env: FOLDER: ${{ github.event.pull_request.number }} + SENTRY_AUTH_TOKEN: ${{ secrets.SENTRY_AUTH_TOKEN }} + SENTRY_ORG: ${{ secrets.SENTRY_ORG }} + SENTRY_PROJECT: ${{ secrets.SENTRY_PROJECT }}
🤖 Fix all issues with AI agents
In @.github/workflows/build-web-image.yaml:
- Around line 18-33: Add an explicit top-level permissions block to the workflow
to grant least-privilege token scopes: in the workflow YAML where the workflow
name and env are defined, add a permissions mapping that includes at minimum
"contents: read", "packages: write" and "artifact-metadata: write" (these cover
docker/login-action -> ghcr push and actions/upload-artifact). Place this
permissions block at the same level as "env" and "jobs" so the "build" job
inherits it.
- Around line 70-80: The Sentry release creation step using `npx `@sentry/cli`
releases new "$SENTRY_RELEASE"` is not idempotent and will fail on reruns if the
release already exists; update the workflow step that uses the SENTRY_RELEASE
env and `npx `@sentry/cli` releases new` to first check for the release (e.g.,
call `sentry-cli releases info "$SENTRY_RELEASE"`) and only create it when it
does not exist, ensuring the subsequent `releases files` and `releases finalize`
commands run regardless of prior runs.
🧹 Nitpick comments (1)
.github/actions/build-flutter-web/action.yml (1)
50-55: Harden version parsing against inline comments.
Ifpubspec.yamlever includesversion: 1.2.3 # comment, the current whitespace stripping would fold the comment into the version string. Stripping inline comments first keeps this robust.♻️ Suggested tweak
- VERSION=$(grep '^version:' pubspec.yaml | sed 's/version: //;s/[[:space:]]//g') + VERSION=$(grep '^version:' pubspec.yaml | sed -E 's/^version:[[:space:]]*//; s/[[:space:]]+#.*$//; s/[[:space:]]//g')
| # Upload source maps to Sentry via sentry-cli | ||
| - name: Upload source maps to Sentry | ||
| if: env.SENTRY_AUTH_TOKEN != '' | ||
| env: | ||
| SENTRY_RELEASE: ${{ steps.version.outputs.VERSION }} | ||
| run: | | ||
| npx @sentry/cli releases new "$SENTRY_RELEASE" | ||
| npx @sentry/cli releases files "$SENTRY_RELEASE" \ | ||
| upload-sourcemaps ./source-maps/web/ \ | ||
| --url-prefix '~/' | ||
| npx @sentry/cli releases finalize "$SENTRY_RELEASE" |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
sentry-cli releases new idempotent duplicate release already exists
💡 Result:
sentry-cli releases new <VERSION> is not idempotent: if the release already exists, the command can fail with a “release already exists”/duplicate-type API error, and you need to handle that in your pipeline yourself. The most recent sentry-cli release (3.1.0, released Jan 12, 2026) does not mention adding an idempotent “create-if-missing” behavior for releases new in its notes/changelog. [1][2]
Practical ways to make it idempotent in CI
Option A (recommended): check first, then create
if ! sentry-cli releases info "$SENTRY_RELEASE" >/dev/null 2>&1; then
sentry-cli releases new -p "$SENTRY_PROJECT" "$SENTRY_RELEASE"
fiOption B: ignore the “already exists” failure
sentry-cli releases new -p "$SENTRY_PROJECT" "$SENTRY_RELEASE" || trueSentry’s own CI examples still show a plain sentry-cli releases new ... as the “create release” step (which will fail if you rerun the same release without guarding it). [3]
Sources: [1] [2] [3]
🌐 Web query:
sentry-cli releases new --force flag documentation
💡 Result:
The current official Sentry CLI “Release Management” docs describe sentry-cli releases new (and mention --finalize), but do not document any --force flag for releases new. [1]
If your installed sentry-cli shows a --force option, the only official place you can reliably find its up-to-date meaning/behavior is the built-in CLI help:
sentry-cli releases new --helpSentry’s CLI docs explicitly recommend using the help command to see documentation for all parameters. [2]
Sources: [1] [2]
Add idempotent release creation for Sentry reruns.
sentry-cli releases new fails if the release already exists, making this step non-idempotent. Reruns with the same version will fail. Guard the command by checking if the release exists first:
Suggested fix
sentry-cli releases info "$SENTRY_RELEASE" >/dev/null 2>&1 || \
npx `@sentry/cli` releases new "$SENTRY_RELEASE"Alternatively, suppress the error with || true, but checking existence first is safer.
🤖 Prompt for AI Agents
In @.github/workflows/build-web-image.yaml around lines 70 - 80, The Sentry
release creation step using `npx `@sentry/cli` releases new "$SENTRY_RELEASE"` is
not idempotent and will fail on reruns if the release already exists; update the
workflow step that uses the SENTRY_RELEASE env and `npx `@sentry/cli` releases
new` to first check for the release (e.g., call `sentry-cli releases info
"$SENTRY_RELEASE"`) and only create it when it does not exist, ensuring the
subsequent `releases files` and `releases finalize` commands run regardless of
prior runs.
|
Is this work still needed if the source maps are directly onto the static assets? If so, please set up a workshop with @Crash-- to arbitrate this. |
|
Yes, still need to upload source map, no way of Sentry can auto fetch public source map. |
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
We tried to
|
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Fix all issues with AI agents
In @.github/workflows/build-web-image.yaml:
- Around line 18-28: The workflow file contains duplicate top-level keys "name"
and "permissions" which is invalid YAML; remove the duplicate entries so there
is only one name key (keep "Build and push web Docker image") and a single
permissions block, merging permissions into that block (contents: read,
packages: write, actions: write, artifact-metadata: write) so no permission
lines are lost; update the file by deleting the redundant name/permissions
section and ensuring only the consolidated permissions block remains.
- Around line 1-17: The workflow is missing a on.workflow_call.secrets
declaration so callers cannot pass secrets referenced via ${{ secrets.* }}
(causing Sentry upload and DockerHub login to fail); add an
on.workflow_call.secrets block listing each secret name the workflow uses (e.g.,
the Sentry token and DockerHub credentials or any other secrets referenced),
using the existing on.workflow_call structure so callers can supply those
secrets when invoking the reusable workflow.
| on: | ||
| workflow_call: | ||
| inputs: | ||
| environment: | ||
| description: "Deployment environment (dev/prod)" | ||
| type: string | ||
| required: true | ||
| docker-tags: | ||
| description: "Docker metadata tags (passed to docker/metadata-action)" | ||
| type: string | ||
| required: true | ||
| artifact-name: | ||
| description: "Name for the source map artifact" | ||
| type: string | ||
| required: false | ||
| default: "source-maps" | ||
|
|
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
GitHub Actions reusable workflows secrets declaration workflow_call required
💡 Result:
To make a secret required for a reusable workflow, declare it under on.workflow_call.secrets in the called workflow and set required: true:
# .github/workflows/reusable.yml (called workflow)
on:
workflow_call:
secrets:
personal_access_token:
required: true
jobs:
example:
runs-on: ubuntu-latest
steps:
- run: echo "Got it"
env:
TOKEN: ${{ secrets.personal_access_token }}Then, in the caller workflow, pass that secret using jobs.<job_id>.secrets (names must match the declaration), otherwise the call fails because it’s required:
# .github/workflows/caller.yml (caller workflow)
jobs:
call:
uses: org/repo/.github/workflows/reusable.yml@main
secrets:
personal_access_token: ${{ secrets.PERSONAL_ACCESS_TOKEN }}Notes:
- Passing a secret not declared in the called workflow causes an error. [2]
- You can also pass all caller secrets with
secrets: inherit. [1][2]
Sources: GitHub Docs on reusable workflows and workflow_call secrets. [1][2]
🏁 Script executed:
# First, let's check if this file exists and read its complete content
cd /tmp && git -C /tmp/repo rev-parse --show-toplevel 2>/dev/null || echo "Not in git repo"
# Read the complete workflow file to see all secrets references
cat .github/workflows/build-web-image.yamlRepository: linagora/tmail-flutter
Length of output: 169
🏁 Script executed:
# Find the workflow file
fd -type f "build-web-image.yaml"
# Also list files in .github/workflows directory if it exists
ls -la .github/workflows/ 2>/dev/null || echo "Directory not found"Repository: linagora/tmail-flutter
Length of output: 892
🏁 Script executed:
# Read the complete workflow file
cat .github/workflows/build-web-image.yamlRepository: linagora/tmail-flutter
Length of output: 4194
Declare reusable-workflow secrets in on.workflow_call.secrets so they can be properly passed by callers.
This workflow is a reusable workflow (workflow_call). Secrets used via ${{ secrets.* }} must be declared under on.workflow_call.secrets, otherwise callers cannot pass them and they resolve empty. This causes the Sentry upload and DockerHub login steps to fail.
🔐 Suggested `workflow_call.secrets` block
on:
workflow_call:
inputs:
environment:
description: "Deployment environment (dev/prod)"
type: string
required: true
docker-tags:
description: "Docker metadata tags (passed to docker/metadata-action)"
type: string
required: true
artifact-name:
description: "Name for the source map artifact"
type: string
required: false
default: "source-maps"
+ secrets:
+ SENTRY_AUTH_TOKEN:
+ required: false
+ SENTRY_ORG:
+ required: false
+ SENTRY_PROJECT:
+ required: false
+ DOCKERHUB_USERNAME:
+ required: true
+ DOCKERHUB_TOKEN:
+ required: true🤖 Prompt for AI Agents
In @.github/workflows/build-web-image.yaml around lines 1 - 17, The workflow is
missing a on.workflow_call.secrets declaration so callers cannot pass secrets
referenced via ${{ secrets.* }} (causing Sentry upload and DockerHub login to
fail); add an on.workflow_call.secrets block listing each secret name the
workflow uses (e.g., the Sentry token and DockerHub credentials or any other
secrets referenced), using the existing on.workflow_call structure so callers
can supply those secrets when invoking the reusable workflow.
| name: Build and push web Docker image | ||
|
|
||
| permissions: | ||
| contents: read | ||
| name: Build and push web Docker image | ||
|
|
||
| permissions: | ||
| contents: read | ||
| packages: write | ||
| actions: write | ||
| artifact-metadata: write |
There was a problem hiding this comment.
Remove duplicate name/permissions keys (invalid YAML).
Duplicate keys are invalid YAML and will fail linting; GitHub may reject the workflow file. Keep a single name and a single permissions block.
🧹 Suggested consolidation
-name: Build and push web Docker image
-
-permissions:
- contents: read
-name: Build and push web Docker image
-
-permissions:
- contents: read
- packages: write
- actions: write
- artifact-metadata: write
+name: Build and push web Docker image
+
+permissions:
+ contents: read
+ packages: write
+ actions: write
+ artifact-metadata: write📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| name: Build and push web Docker image | |
| permissions: | |
| contents: read | |
| name: Build and push web Docker image | |
| permissions: | |
| contents: read | |
| packages: write | |
| actions: write | |
| artifact-metadata: write | |
| name: Build and push web Docker image | |
| permissions: | |
| contents: read | |
| packages: write | |
| actions: write | |
| artifact-metadata: write |
🧰 Tools
🪛 actionlint (1.7.10)
22-22: key "name" is duplicated in "workflow" section. previously defined at line:18,col:1
(syntax-check)
24-24: key "permissions" is duplicated in "workflow" section. previously defined at line:20,col:1
(syntax-check)
🪛 YAMLlint (1.38.0)
[error] 22-22: duplication of key "name" in mapping
(key-duplicates)
[error] 24-24: duplication of key "permissions" in mapping
(key-duplicates)
🤖 Prompt for AI Agents
In @.github/workflows/build-web-image.yaml around lines 18 - 28, The workflow
file contains duplicate top-level keys "name" and "permissions" which is invalid
YAML; remove the duplicate entries so there is only one name key (keep "Build
and push web Docker image") and a single permissions block, merging permissions
into that block (contents: read, packages: write, actions: write,
artifact-metadata: write) so no permission lines are lost; update the file by
deleting the redundant name/permissions section and ensuring only the
consolidated permissions block remains.
|
|
||
| - Minified JS in Sentry error reports is unreadable without source maps | ||
| - Source maps must not ship to production (exposes original code) | ||
| - Source maps must match the exact build deployed |
There was a problem hiding this comment.
Please add "Sentry do not support using the website source maps but requires to upload it" potentially if possible with a ink justifying that choice on their end.
Too bad :-( That's unfortunate. Thanks for the clarification However we then need a very clear documentation for a Twake Mail operator (think @dungphampt ) given XYZ twake mail flutter image, have VERY CLEAR instructions on how to upload the source maps onto Sentry. THis needs to be documented in the sentry documentation page and haveing the build artifact (the docker image/container) shall be sufficiant to conduct this task - no need to recompile. (Not just the ADR that is a good first step) |
|
Yeah for sentry there is 2 mains reasons:
But yes, uploading source map to sentry adds complexity for the deploiement. That said, I agree the contract can be made clearer. What do you do:
(The "mental model" of sentry works better for mobile app. ) |
|
addressed in #4311 |
5 participants
Resolved
Summary by CodeRabbit
Infrastructure & Build Improvements
New Features
Chores
Documentation
✏️ Tip: You can customize this high-level summary in your review settings.