Skip to content

Conversation

@ErkoRisthein
Copy link

Summary

Fixes TypeError: Cannot read properties of undefined (reading 'startsWith') that occurs when the window receives postMessage events that have a data property but no action property.

Root Cause

The condition event.data?.action.startsWith("web-eid:") safely handles when event.data is undefined, but throws when event.data exists and event.data.action is undefined.

Fix

Add optional chaining on action:

if (!event.data?.action?.startsWith("web-eid:")) return;

Test Plan

  • Added unit tests for messages with missing action property
  • npm test passes (48 tests)

Signed-off-by: Erko Risthein erko@risthein.ee

Add optional chaining on action property to prevent "Cannot read
properties of undefined (reading 'startsWith')" error when window
receives messages that have data but no action property.
Copilot AI review requested due to automatic review settings December 11, 2025 10:14
Copy link

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 fixes a TypeError that occurs when the window receives postMessage events with a data property but without an action property. The fix adds optional chaining to safely access the action property before calling startsWith().

Key Changes:

  • Added optional chaining operator to prevent TypeError when event.data.action is undefined
  • Added unit tests to verify messages without the action property are properly ignored

Reviewed changes

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

File Description
src/services/WebExtensionService.ts Added optional chaining to event.data.action to prevent TypeError when action is undefined
src/services/tests/WebExtensionService-test.ts Added three test cases verifying that messages with missing/null/undefined data or action properties are properly ignored

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

Verify that console.warn is not called when messages are ignored,
rather than just checking that no error is thrown.
Copy link

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

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


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

@taneltm
Copy link
Member

taneltm commented Jan 5, 2026

Thanks for noticing the issue!

Made this mistake when I changed:

    if (!/^web-eid:/.test(event.data?.action)) return;

to

    if (!event.data?.action.startsWith("web-eid:")) return;

Your change does fix the issue, but only if we expect event.data.action to always be a string.
If a website or extension had action as an object, it would still cause an error.
For example:

window.postMessage(
    {
        action: {
            id: "<actionId>",
            _t: "<actionTimestamp>",
        },
        payload: {
            ...
        }
    },
    "*"
);

We could add another optional chaining check, so ...action?.startsWith?.("web-eid:"), but if we don't check the action type nor startsWith type, we could still end up with an error - Uncaught TypeError: event.data.action.startsWith is not a function.
For example, if someone uses the startsWith as a property name.

window.postMessage(
    {
        action: {
            startsWith: "2022-10-12",
            endsWith: "2026-10-12
        }
    },
    "*"
);

Adding type checks would make this IF statement harder to read at a glance, so I think the best option would be to revert it back to:

    if (!/^web-eid:/.test(event.data?.action)) return;

The regex .test(...) accepts any data type, but only matches strings.

What do you think, @ErkoRisthein ?

Address code review feedback:
- Add typeof check to reject non-string action values early
- Use startsWith() safely after confirming action is a string
- Add test cases for action as object, number, and array
@ErkoRisthein
Copy link
Author

Thanks for the review, @taneltm. You're right that the optional chaining approach doesn't handle all edge cases.

I tried reverting to the regex approach you suggested:

if (!/^web-eid:/.test(event.data?.action)) return;

However, I discovered a non-intuitive edge case: JavaScript's RegExp.test() coerces its argument to a string before matching. This means an array like ["web-eid:test"] gets coerced to "web-eid:test" and passes the regex check. The code then fails later at message.action.endsWith() since arrays don't have that method.

So I've added an explicit type guard before calling startsWith:

if (typeof event.data?.action !== "string") return;
if (!event.data.action.startsWith("web-eid:")) return;

This handles all edge cases:

  • Objects like { id: "123" } are rejected by type check
  • Objects with startsWith property like { startsWith: "2022-10-12" } are rejected by type check
  • Arrays like ["web-eid:test"] are rejected by type check
  • Numbers, null, undefined are all rejected by type check

I've added test cases for these scenarios. All tests pass.

Copy link
Member

@taneltm taneltm left a comment

Choose a reason for hiding this comment

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

  • Original issue reproduced in main
  • PR fixes the issue
  • Confirmed that the tests are working properly (tests fail, as expected, when the fix is reverted)

@mrts
Copy link
Member

mrts commented Jan 6, 2026

@ErkoRisthein, heartfelt thanks for your contribution!

A couple of formalities still remain, then I will gladly merge this: can you please squash the commits into one, sign the Certificate of Origin with git commit --amend --sign-off and do a force push? Thanks in advance!

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