Skip to content

Fix 500 in payment status read when a concurrent webhook locks the data element#1804

Merged
Magnusrm merged 2 commits into
mainfrom
fix/payment-status-read-409-locked-data
Jun 11, 2026
Merged

Fix 500 in payment status read when a concurrent webhook locks the data element#1804
Magnusrm merged 2 commits into
mainfrom
fix/payment-status-read-409-locked-data

Conversation

@Magnusrm

@Magnusrm Magnusrm commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Description

Problem

GET …/payment (GetPaymentInformation) intermittently returned 500 after a successful payment.

The endpoint persists the latest payment status on read when the requested task is the current task. This races with the payment webhook, which advances the process and locks the payment data element. The persist PUT to storage then fails with 409 - "data element <id> is locked and cannot be updated", which surfaced to the user as a 500 — typically right after returning from the hosted payment page.

Why the existing mitigation missed it

GetPaymentInformation already guarded this race: on a persist failure it called CurrentTaskMovedAwayFrom and only degraded to a read-only result if it could confirm the current task had moved. But the data-element lock can become visible before the CurrentTask change does, so in that window the re-check still saw the payment task as current → the 409 was rethrown → 500. This made it intermittent ("some users").

Fix

Treat a 409/Conflict from t back to read-only paymentstatus, without requiring the task-moved check — the lock itself is the authoritative signal that a c) already finalized things.Non-409 exceptions keep their previous behavior (still propagate when the taskmoved).

Testing

Adds a regression test,GetPaymentInformation_PersistFailsWithLockedDataElement_FallsBackToReadOnly,reproduces the production scel storage needed): the persist throws PlatformHttpException(409, "…is locked…") with the current task left unchanged, and the endpoithe read-only status. Theexisting PersistFailsButTaskUnchanged_PropagatesException test confirms non-409 errors
still propagate.

🤖 Generated with Claude Code

Frontend PR adding retries in order to further harden the process-flow:
Altinn/app-frontend-react#4259

Related Issue(s)

  • #{issue number}

Verification

  • Your code builds clean without any errors or warnings
  • Manual testing done (required)
  • Relevant automated test added (if you find this hard, leave it and we'll help out)
  • All tests run green

Documentation

  • User documentation is updated with a separate linked PR in altinn-studio-docs. (if applicable)

@coderabbitai

coderabbitai Bot commented Jun 10, 2026

Copy link
Copy Markdown
Contributor

Warning

Review limit reached

@Magnusrm, we couldn't start this review because you've reached your PR review rate limit.

More reviews will be available in 56 minutes and 44 seconds. Learn how PR review limits work.

Your organization has run out of usage credits. Purchase more credits in the billing tab to continue.

⌛ How to resolve this issue?

After more reviews become available, a review can be triggered using the @coderabbitai review command as a PR comment. Alternatively, push new commits to this PR.

We recommend that you space out your commits to avoid hitting the rate limit.

🚦 How do rate limits work?

CodeRabbit enforces hourly rate limits for each developer per organization.

Our paid plans include higher PR review limits than trial, open-source, and free plans. In all cases, reviews become available again over time. During sustained high-volume PR review activity, CodeRabbit may temporarily slow when the next review becomes available.

Please see our Fair Usage Limits Policy for further information.

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: 82f4f070-dd31-480b-a7d7-fc71baf10659

📥 Commits

Reviewing files that changed from the base of the PR and between b86fb54 and 06a2876.

📒 Files selected for processing (2)
  • src/Altinn.App.Api/Controllers/PaymentController.cs
  • test/Altinn.App.Api.Tests/Controllers/PaymentControllerTests.cs
✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch fix/payment-status-read-409-locked-data

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.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@Magnusrm Magnusrm added the bugfix Label Pull requests with bugfix. Used when generation releasenotes label Jun 10, 2026
@sonarqubecloud

Copy link
Copy Markdown

@Magnusrm Magnusrm marked this pull request as ready for review June 11, 2026 09:29
@Magnusrm Magnusrm moved this to 🔎 In review in Team Altinn Studio Jun 11, 2026

@ivarne ivarne left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Looks very reasonable!

Maybe @vxkc could also have a look?

@Magnusrm Magnusrm changed the title Fix 500 in payment status read when a concurrent webhook locks the data element' Fix 500 in payment status read when a concurrent webhook locks the data element Jun 11, 2026
@Magnusrm Magnusrm merged commit 0dc935c into main Jun 11, 2026
16 of 17 checks passed
@Magnusrm Magnusrm deleted the fix/payment-status-read-409-locked-data branch June 11, 2026 10:17
@github-project-automation github-project-automation Bot moved this from 🔎 In review to ✅ Done in Team Altinn Studio Jun 11, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Label Pull requests with bugfix. Used when generation releasenotes

Projects

Status: ✅ Done

Development

Successfully merging this pull request may close these issues.

3 participants