Skip to content

[CURA-13047] 'findFieldByName' now gives null on Windows, but the name exists#16

Merged
HellAholic merged 1 commit intomainfrom
CURA-13047_post_upgrade_slice_crash
Mar 25, 2026
Merged

[CURA-13047] 'findFieldByName' now gives null on Windows, but the name exists#16
HellAholic merged 1 commit intomainfrom
CURA-13047_post_upgrade_slice_crash

Conversation

@rburema
Copy link
Copy Markdown
Member

@rburema rburema commented Mar 18, 2026

This then causes a crash in the error-handling routine (not fixed by this commit), because that triggers when the returned field is a nullptr.

(This happened after we upgraded protobuf, and exclusively on Windows.)

Proposed solution, under discussion, since it's quite hacky, and doesn't solve the underlying problem (whatever that actually is).

…ough the name exists.

This then causes a crash in the error-handling routine (not fixed by this commit), because that triggers when the returned field is a nullptr.

proposed solution w.r.t. CURA-13047
Copy link
Copy Markdown
Contributor

@wawanbreton wawanbreton left a comment

Choose a reason for hiding this comment

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

Hmm, I would really like to know more about the root cause of the issue before accepting this fix... It really feels like we are missing something obvious that we should do on our side now.
Also you mention "a crash in the error-handling routine (not fixed by this commit)", so that means if it ever happens that a field is missing, it will still crash ? This is a bit disappointing since the "error-handling routine" is supposed to properly fallback when such a case happens 😅

@rburema
Copy link
Copy Markdown
Member Author

rburema commented Mar 24, 2026

@wawanbreton

Hmm, I would really like to know more about the root cause of the issue before accepting this fix... It really feels like we are missing something obvious that we should do on our side now.

I could dive into this more deeply, but that would take up time we may not have; let's discuss when you're in the office.

Also you mention "a crash in the error-handling routine (not fixed by this commit)", so that means if it ever happens that a field is missing, it will still crash ? This is a bit disappointing since the "error-handling routine" is supposed to properly fallback when such a case happens 😅

So that's 'just' the set-error bit before the return null -- that is, it sets error information. (The whole return null bit works fine if you remove the 'error handling'.) The reason I didn't fix that is mostly that's done everywhere in this repository; that's how you set the error-information. Secondly, even if I did outright remove that bit (which is honestly a way we may go), we'd still have a crash in the front-end (albeit one that gets you a stack-trace.)

@HellAholic
Copy link
Copy Markdown
Contributor

HellAholic commented Mar 25, 2026

Temp solution to unblock the process of development. Follow up ticket to address properly for internal reference:

CURA-13058

@HellAholic HellAholic merged commit 52d1367 into main Mar 25, 2026
6 checks passed
@HellAholic HellAholic deleted the CURA-13047_post_upgrade_slice_crash branch March 25, 2026 10:07
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