Skip to content

Stop reporting expected student validation errors to Sentry#938

Open
abcampo-iry wants to merge 2 commits into
mainfrom
issues/1583
Open

Stop reporting expected student validation errors to Sentry#938
abcampo-iry wants to merge 2 commits into
mainfrom
issues/1583

Conversation

@abcampo-iry

@abcampo-iry abcampo-iry commented Jul 24, 2026

Copy link
Copy Markdown
Contributor

Related to Sentry issue https://github.com/RaspberryPiFoundation/digital-editor-issues/issues/1583

Points for consideration:

  • No security impact.
  • No performance impact.

What's changed?

  • When someone tries to create a student with:
    • Username already taken
    • Password matches the username
    • Password too short
    • Missing or invalid student details

Profile API returns an isComplex validation error.

  • This is an expected error that is shown to the user, but we were also sending it to Sentry.
  • This change handles student validation errors separately, so they are no longer sent to Sentry (since i don't think they add value and cause noise)..

Steps to perform after deploying to production

No additional steps are required. There are no migrations, configuration changes, or data backfills.

Copilot AI review requested due to automatic review settings July 24, 2026 13:29
@cla-bot cla-bot Bot added the cla-signed label Jul 24, 2026
Comment thread lib/concepts/school_student/create.rb Fixed

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR updates the SchoolStudent::Create concept to treat Profile API “student validation” failures as expected outcomes rather than unexpected exceptions, reducing Sentry noise when user-facing validation fails during student creation.

Changes:

  • Add a dedicated rescue for ProfileApiClient::Student422Error to avoid reporting expected validation failures to Sentry.
  • Adjust unit specs to assert the returned error is the translatable errorCode and that Sentry is not invoked for this case.

Reviewed changes

Copilot reviewed 2 out of 2 changed files in this pull request and generated 1 comment.

File Description
spec/concepts/school_student/create_spec.rb Updates expectations for Profile API validation failures (returns errorCode, does not call Sentry).
lib/concepts/school_student/create.rb Adds a specific rescue for Student422Error so expected validation errors aren’t sent to Sentry.

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

Comment thread lib/concepts/school_student/create.rb Outdated
@github-actions

github-actions Bot commented Jul 24, 2026

Copy link
Copy Markdown

Test coverage

92.17% line coverage reported by SimpleCov.
Run: https://github.com/RaspberryPiFoundation/editor-api/actions/runs/30097655008

Copilot AI review requested due to automatic review settings July 24, 2026 13:37

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Copilot reviewed 2 out of 2 changed files in this pull request and generated no new comments.

Comments suppressed due to low confidence (1)

lib/concepts/school_student/create.rb:13

  • This rescue path relies on e.to_s to return the translatable Profile validation code. That happens today because Student422Error sets its exception message from errorCode, but it’s an indirect coupling (and can degrade to the exception class name if the payload shape changes). Prefer extracting errorCode directly from e.errors and only fall back to to_s when it’s missing.
      rescue ProfileApiClient::Student422Error => e
        OperationResponse[error: e.to_s]
      rescue StandardError => e
        Sentry.capture_exception(e)
        OperationResponse[error: e.to_s]

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants