Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[HOLD for Payment 10/8] [$250] [Crashlytics] TypeError: undefined is not a function #46117

Closed
CortneyOfstad opened this issue Jul 24, 2024 · 51 comments
Assignees
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2

Comments

@CortneyOfstad
Copy link
Contributor

CortneyOfstad commented Jul 24, 2024

Coming from this GH — #45054 (comment)

Reported by @TMisiukiewicz

Fatal Exception: com.facebook.react.common.JavascriptException: TypeError: undefined is not a function, js engine: hermes, stack:
init@1:2323334
_default@1:7673375
anonymous@1:180908
loadModuleImplementation@1:170989
guardedLoadModule@1:170179
metroRequire@1:169506
global@1:168441

       at com.facebook.react.modules.core.ExceptionsManagerModule.reportException(ExceptionsManagerModule.java:65)
       at com.facebook.jni.NativeRunnable.run(NativeRunnable.java)
       at android.os.Handler.handleCallback(Handler.java:958)
       at android.os.Handler.dispatchMessage(Handler.java:99)
       at com.facebook.react.bridge.queue.MessageQueueThreadHandler.dispatchMessage(MessageQueueThreadHandler.java:29)
       at android.os.Looper.loopOnce(Looper.java:230)
       at android.os.Looper.loop(Looper.java:319)
       at com.facebook.react.bridge.queue.MessageQueueThreadImpl$4.run(MessageQueueThreadImpl.java:234)
       at java.lang.Thread.run(Thread.java:1012)
Upwork Automation - Do Not Edit
  • Upwork Job URL: https://www.upwork.com/jobs/~0147e5efa631c195d0
  • Upwork Job ID: 1829521509135781835
  • Last Price Increase: 2024-08-30
Issue OwnerCurrent Issue Owner: @paultsimura
@CortneyOfstad CortneyOfstad self-assigned this Jul 24, 2024
@CortneyOfstad CortneyOfstad added Daily KSv2 Bug Something is broken. Auto assigns a BugZero manager. labels Jul 24, 2024
Copy link

melvin-bot bot commented Jul 24, 2024

Current assignee @CortneyOfstad is eligible for the Bug assigner, not assigning anyone new.

Copy link

melvin-bot bot commented Jul 29, 2024

@CortneyOfstad Huh... This is 4 days overdue. Who can take care of this?

@CortneyOfstad
Copy link
Contributor Author

@TMisiukiewicz it was indicated on one of the other Crashlytic GHs that the logs are truncated. Any way you could pull up the full logs? Thanks!

@melvin-bot melvin-bot bot added Overdue and removed Overdue labels Jul 31, 2024
Copy link

melvin-bot bot commented Aug 5, 2024

@CortneyOfstad Eep! 4 days overdue now. Issues have feelings too...

@TMisiukiewicz
Copy link
Contributor

can we change it to weekly?

Copy link

melvin-bot bot commented Aug 7, 2024

@CortneyOfstad this issue was created 2 weeks ago. Are we close to a solution? Let's make sure we're treating this as a top priority. Don't hesitate to create a thread in #expensify-open-source to align faster in real time. Thanks!

Copy link

melvin-bot bot commented Aug 7, 2024

@CortneyOfstad Still overdue 6 days?! Let's take care of this!

Copy link

melvin-bot bot commented Aug 9, 2024

@CortneyOfstad 8 days overdue is a lot. Should this be a Weekly issue? If so, feel free to change it!

@CortneyOfstad
Copy link
Contributor Author

Sorry, was OoO, so adjusting the frequency now!

@melvin-bot melvin-bot bot removed the Overdue label Aug 12, 2024
@CortneyOfstad CortneyOfstad added Weekly KSv2 and removed Daily KSv2 labels Aug 12, 2024
@melvin-bot melvin-bot bot added the Overdue label Aug 20, 2024
@hurali97
Copy link
Contributor

Hey @CortneyOfstad 👋

To reproduce this I went through the Logs and Breadcrumbs section here in firebase crashlytics but I couldn't reproduce it. Also, for another similar event the logs and breadcrumbs were different, so I am not sure whether following are a correct reproduction steps but I am writing these from the link above:

  • Create Submit Expense
  • Set distance
  • Select participants OR workspace
  • Submit Expense
  • Open the generated expense and view the details

After the last step, we get an error in firebase crashlytics. I tried it but I couldn't reproduce it.


Now, let's try to find the root cause in the code using the help from the stack trace. The function which is causing this error is getExtension which is a function in Str.ts in expensify-common. I opted to test this by isolating this function and below are the results:

Screenshot 2024-08-20 at 5 41 32 PM

We get an error url.split is not a function, which is similar to the error we get in crashlytics. This error only happens if the passed url is of another type than string, precisely it's of type number or array. On any other type like null or undefined, we get a different error.

To fix this, we can add a typeof operator in getExtension function, which will early return if url is not of string type. Below is the diff of suggested changes:

diff --git a/lib/str.ts b/lib/str.ts
index e1dd37c..432f8ed 100644
--- a/lib/str.ts
+++ b/lib/str.ts
@@ -992,6 +992,7 @@ const Str = {
      * without query parameters
      */
     getExtension(url: string): string | undefined {
+        if (typeof url !== 'string') return undefined;
         return url.split('.').pop()?.split('?')[0]?.toLowerCase();
     },
 

@CortneyOfstad
Copy link
Contributor Author

Thanks for the detailed explanation @hurali97!

@TMisiukiewicz reviewing the comment above, there may potentially be an issue in the breadcrumb flow from the original issue that this was created from. Do you have any insight into what could potentially be missing or needs to be adjusted in order to recreate?

@melvin-bot melvin-bot bot removed the Overdue label Aug 21, 2024
@TMisiukiewicz
Copy link
Contributor

@CortneyOfstad currently breadcrumbs are tied only to navigation events only, and this, unfortunately, it gaves us very limited knowledge about users actions. When I have a bit more capacity, I am planning to review if there is a space for improvements in this area.

Also, the proposal from @hurali97 looks good to me 👍

@hurali97
Copy link
Contributor

@CortneyOfstad do you think we can move forward with creating a PR to fix this?

@melvin-bot melvin-bot bot added the Overdue label Aug 29, 2024
@CortneyOfstad
Copy link
Contributor Author

Sorry for the delay here — PR looks great! Thank you!

@melvin-bot melvin-bot bot removed the Overdue label Aug 30, 2024
@CortneyOfstad CortneyOfstad added the External Added to denote the issue can be worked on by a contributor label Aug 30, 2024
@melvin-bot melvin-bot bot added the Overdue label Sep 9, 2024
@paultsimura
Copy link
Contributor

There is an ongoing discussion in the PR

@melvin-bot melvin-bot bot removed the Overdue label Sep 9, 2024
Copy link

melvin-bot bot commented Sep 13, 2024

@francoisl, @CortneyOfstad, @paultsimura, @hurali97 Whoops! This issue is 2 days overdue. Let's get this updated quick!

@melvin-bot melvin-bot bot added the Overdue label Sep 13, 2024
@francoisl
Copy link
Contributor

Oh, we still need a PR to bump the expensify-common version in package.json, @hurali97 do you mind sending one please?

@deetergp
Copy link
Contributor

We are about to bump to 2.0.88 in this PR. Do you want us to wait, or just pull in your changes with ours?

Copy link

melvin-bot bot commented Sep 17, 2024

@francoisl, @CortneyOfstad, @paultsimura, @hurali97 Still overdue 6 days?! Let's take care of this!

@paultsimura
Copy link
Contributor

@deetergp I think you can pull them in. There is nothing breaking here – just some extra precautions & logging.

@melvin-bot melvin-bot bot removed the Overdue label Sep 18, 2024
Copy link

melvin-bot bot commented Sep 23, 2024

@francoisl, @CortneyOfstad, @paultsimura, @hurali97 Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Sep 23, 2024
@francoisl
Copy link
Contributor

Still waiting on #49038 to be merged

@paultsimura
Copy link
Contributor

Not overdue.

@melvin-bot melvin-bot bot removed the Overdue label Sep 25, 2024
Copy link

melvin-bot bot commented Sep 30, 2024

@francoisl, @CortneyOfstad, @paultsimura, @hurali97 Eep! 4 days overdue now. Issues have feelings too...

@melvin-bot melvin-bot bot added the Overdue label Sep 30, 2024
@paultsimura
Copy link
Contributor

@francoisl #49038 was merged 4 days ago. What's the plan here now?

@melvin-bot melvin-bot bot removed the Overdue label Sep 30, 2024
@francoisl
Copy link
Contributor

Ah nice, and it was just deployed to production too.

I think we can issue payments and close this. With the new Log.warn, if the issue shows up again, we'll see it in our logs and an internal engineer will be assigned to investigate.

@paultsimura
Copy link
Contributor

The corresponding version bump was deployed to production on Oct 1. Payment is due on 2024-10-08.

@francoisl francoisl added Weekly KSv2 and removed Daily KSv2 labels Oct 1, 2024
@paultsimura
Copy link
Contributor

I don't think this GH is suitable for a BZ checklist: it is more like a preventive change with extra logging to detect and avoid previously uncaught crashes.

@CortneyOfstad
Copy link
Contributor Author

@paultsimura — i sent you an offer in Upwork here. Please let me know once you accept, and I'll get that paid ASAP. Thanks!

@CortneyOfstad CortneyOfstad changed the title [$250] [Crashlytics] TypeError: undefined is not a function [HOLD for Payment 10/8] [$250] [Crashlytics] TypeError: undefined is not a function Oct 8, 2024
@paultsimura
Copy link
Contributor

Accepted, thanks @CortneyOfstad

@CortneyOfstad
Copy link
Contributor Author

Payment Summary

@paultsimura — paid $250 via Upwork
@hurali97 — contracted position (CallStack)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug Something is broken. Auto assigns a BugZero manager. External Added to denote the issue can be worked on by a contributor Weekly KSv2
Projects
Development

No branches or pull requests

7 participants