Skip to content

RO-3171: Fiks race-condition feil#953

Open
jorgkv wants to merge 2 commits intodevelopfrom
fix/RO-3171-plans-init-race-condition
Open

RO-3171: Fiks race-condition feil#953
jorgkv wants to merge 2 commits intodevelopfrom
fix/RO-3171-plans-init-race-condition

Conversation

@jorgkv
Copy link
Copy Markdown
Contributor

@jorgkv jorgkv commented Mar 24, 2026

Jeg tror feilen skyldtes at map.component hentet turer via metadata-egenskapen på GeoJSONService under oppstarten, uten at det ble sjekket om servicen var ferdig med initialisering / lesing fra databasen. I de tilfellene hvor MapComponent leste metadata før GeoJSON var klar, vil i alle fall toggling av og på være eneste fix, og stenge/starte appen igjen ikke nødvendigvis hjelpe, så det stemmer bra med det @gruble opplevde.

Denne fiksen sørger for at appen venter med oppstarten til GeoJSONService.init er ferdig.
Merk at jeg ikke har fått feilen selv, så jeg er ikke sikker på om feilen løses av dette.

Jeg tror feilen skyldtes at map.component hentet turer via metadata-egenskapen på GeoJSONService under oppstarten, uten at det ble sjekket om servicen var ferdig med initialisering / lesing fra databasen.
I de tilfellene hvor MapComponent leste metadata før GeoJSON var klar, vil i alle fall toggling av og på være eneste fix, og stenge/starte appen igjen ikke nødvendigvis hjelpe, så det stemmer bra med det @gruble opplevde.

Denne fiksen sørger for at appen venter med oppstarten til GeoJSONService.init er ferdig.
@github-actions
Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://victorious-water-056410803-953.westeurope.azurestaticapps.net

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

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 aims to fix a startup race condition by ensuring GeoJSON metadata is loaded before the rest of the app proceeds, avoiding cases where MapComponent reads GeoJSONService.metadata() before the database read has completed.

Changes:

  • Stop auto-initializing GeoJSONService in its constructor; expose init() for controlled startup sequencing.
  • Add a global provideAppInitializer that calls GeoJSONService.init() during app bootstrap.
  • Wrap GeoJSON initialization in try/catch to avoid blocking app startup if metadata loading fails.

Reviewed changes

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

File Description
src/app/core/services/geojson/geojson.service.ts Makes initialization explicit and adds error handling around metadata loading.
src/app/app.providers.ts Adds an app initializer to await GeoJSON initialization during bootstrap.

Comment on lines +52 to +54
setTimeout(() => (this.initialized = true)); // For å unngå en første unødvendig lagring i effecten
} catch (error) {
this.logger.error(error, DEBUG_TAG, 'Init error');
Copy link

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

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

init() only flips initialized to true inside the try block. If getMetadata() throws (e.g., storage corruption), the service remains permanently uninitialized, which means the effect() will keep returning early and metadata changes will never be persisted. Consider setting initialized in a finally (or also in the catch) so the service can continue operating even after an init failure.

Suggested change
setTimeout(() => (this.initialized = true)); // For å unngå en første unødvendig lagring i effecten
} catch (error) {
this.logger.error(error, DEBUG_TAG, 'Init error');
} catch (error) {
this.logger.error(error, DEBUG_TAG, 'Init error');
} finally {
// For å unngå en første unødvendig lagring i effecten
setTimeout(() => {
this.initialized = true;
});

Copilot uses AI. Check for mistakes.
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Usikker hva jeg tenker er best her @gruble . Sånn som det er nå vil appen bare lagre ting i minne om init feiler. Hvis vi tillater lagring ved å sette initialized til true i finally eller catch-blokk vil appen kunne overskrive data eller kanskje kræsje andre steder. Hva tenker du?

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.

Tja. Jeg synes ditt scenario om at appen bare lagrer ting i minnet er ok, selv om jeg helt forstår hvordan den vil gjøre det.

@github-actions
Copy link
Copy Markdown

Azure Static Web Apps: Your stage site is ready! Visit it here: https://victorious-water-056410803-953.westeurope.azurestaticapps.net

Copy link
Copy Markdown
Contributor

@gruble gruble left a comment

Choose a reason for hiding this comment

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

Bra du forsøker å løse denne!
Jeg har forsøkt å debugge release/v5.0.15 på tlf. nå. Har gjort en del forsøk på å starte appen på nytt mens jeg debugger, men det virker som den alltid greier å gjøre seg ferdig med initialisering av geojson.service før map.component prøver å kalle geoJSONService.metadata()
Men denne fiksen kan ikke skade!

Comment on lines +52 to +54
setTimeout(() => (this.initialized = true)); // For å unngå en første unødvendig lagring i effecten
} catch (error) {
this.logger.error(error, DEBUG_TAG, 'Init error');
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.

Tja. Jeg synes ditt scenario om at appen bare lagrer ting i minnet er ok, selv om jeg helt forstår hvordan den vil gjøre det.

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