Skip to content

Conversation

@alxndrsn
Copy link
Contributor

@alxndrsn alxndrsn commented Jan 13, 2026

This is a workaround for unclean shutdown handling in Sentry SDK.

See: getodk/central#1347

What has been done to verify that this works as intended?

Tested quite a bit manually locally.

Why is this the best possible solution? Were any other approaches considered?

I think the correct solution is to always call Sentry.close(), but without relevant tests it will be:

  1. hard to enforce Sentry.close() is always called, and
  2. very unhelpful if a call is ever missed

Tests are also tricky to write, as the only reproduction I've found is by updating /etc/hosts, so this bug would potentially need a new suite of tests, or at least an update to the existing standard suite. Ideally these tests would:

  1. call every script in lib/bin/, and
  2. run the odk-central-backend service, and shut it down

and then check that all of the above processes halted within a sensible timeframe.

How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?

Should fix their issue with #1347

Does this change require updates to the API documentation? If so, please update docs/api.yaml as part of this PR.

No.

Before submitting this PR, please make sure you have:

  • run make test and confirmed all checks still pass OR confirm CircleCI build passes
  • verified that any code from external sources are properly credited in comments or that everything is internally sourced

This is a workaround for unclean shutdown handling in Sentry SDK.

See: getodk/central#1347
@alxndrsn alxndrsn marked this pull request as ready for review January 13, 2026 13:13
@yanokwa
Copy link
Member

yanokwa commented Jan 13, 2026

I'm happy with the general approach. Perhaps we can file an issue to re-enable client reports once getsentry/sentry-javascript#18802 is fixed?

@lognaturel
Copy link
Member

lognaturel commented Jan 20, 2026

This has now been fixed and released upstream: getsentry/sentry-javascript#18808

But since we want to release an improvement in a point release and client reports are not that important, I think we should release this small change and file an issue to upgrade Sentry and reintroduce client reports.

@lognaturel
Copy link
Member

It looks like db-migration-tests never finishes and so I can't merge. Any idea what's going on? I see #1732 has the same issue @ktuite. I tried rerunning CI and ended up in the same state.

@lognaturel lognaturel merged commit 700f0a8 into getodk:master Jan 22, 2026
5 checks passed
lognaturel pushed a commit to lognaturel/central-backend that referenced this pull request Jan 27, 2026
This is a workaround for unclean shutdown handling in Sentry SDK.
lognaturel added a commit that referenced this pull request Jan 27, 2026
This is a workaround for unclean shutdown handling in Sentry SDK.

Co-authored-by: Alex Anderson <191496+alxndrsn@users.noreply.github.com>
@alxndrsn alxndrsn deleted the 1347-tempfix branch January 28, 2026 14:46
@alxndrsn
Copy link
Contributor Author

It looks like db-migration-tests never finishes and so I can't merge. Any idea what's going on?

I think this was resolved in Slack, and related to db-migration-tests being added as a required repository check.

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants