[HOLD for payment 2022-02-09] Generic error page on desktop does not include a link to the website.

[mvtglobally]

If you haven’t already, check out our contributing guidelines for onboarding and email [email protected] to request to join our Slack channel!

---

Action Performed:

1. Open the desktop app
2. Allow app to crash or get any page error
3. Notice that the generic error page is missing the following text with the link to switch to Web: "Please try closing and reopening the app or switching to web."

Expected Result:

The generic error page should include a link to website, just like we serve on iOS and Android:

Actual Result:

Error page on desktop does not include the text including a link to new.expensify.com (screenshot below)

Workaround:

None

Platform:

Where is this issue occurring?

• Desktop App

Version Number: 1.1.30-0
Reproducible in staging?: Y
Reproducible in production?:
Logs: https://stackoverflow.com/c/expensify/questions/4856
Notes/Photos/Videos: Any additional supporting documentation

Upwork job URL: https://www.upwork.com/jobs/~01c2090be81cc5aedc
Issue reported by: @roryabraham
Slack conversation: https://expensify.slack.com/archives/C01GTK53T8Q/p1642451766078800

View all open jobs on GitHub

[MelvinBot]

Triggered auto assignment to @sketchydroide (Engineering), see https://stackoverflow.com/c/expensify/questions/4319 for more details.

[parasharrajat]

cc: @mananjadhav

[mananjadhav]

Thanks for tagging. This wasn’t a part of the original requirement if you check the GH issue body #6552 (comment)

But I can pick this up, and here’s my proposal:

1. In GenericErrorPage/ErrorBodyText, I’ll move the current index.native.js into a new component say BaseErrorBodyText
2. Add a new file index.desktop.js
3. Import BaseErrorBodyText and render as is in index.native.js and index.desktop.js

[sketchydroide]

Seems like this is an external, my only question is exactly what link this is, I assume it's www.expensify.com, but it doesn't say anywhere in the GH.
@roryabraham can you please confirm the correct url please :)

[mananjadhav]

@sketchydroide I had worked on the previous task, where this is done for the mobile app and following is the link we're redirecting to:

App/src/pages/GenericErrorPage/ErrorBodyText/index.native.js

Lines 15 to 17 in 7dfe96f

	<TextLink href={CONST.NEW_EXPENSIFY_URL} style={[styles.link]}>
	{props.translate('genericErrorPage.body.helpTextWeb')}
	</TextLink>

Also, marking the comment where its mentioned redirect URL is Homepage.

#6552 (comment)

[sketchydroide]

ok, thanks that sounds good, setting as external

[MelvinBot]

Triggered auto assignment to @trjExpensify (External), see https://stackoverflow.com/c/expensify/questions/8582 for more details.

[trjExpensify]

👋 I'm a tad unclear from the OP what we intend to do here. Where are we adding this link on the page, and why only on desktop?

[mananjadhav]

@trjExpensify The link currently exists for Android and iOS as part of the #6856 (comment) PR. Screenshot from that PR with the text and link highlighted.

Hope this helps.

[MelvinBot]

Triggered auto assignment to Contributor-plus team member for initial proposal review - @parasharrajat (Exported)

[MelvinBot]

Triggered auto assignment to @NikkiWines (Exported), see https://stackoverflow.com/c/expensify/questions/7972 for more details.

[trjExpensify]

Ah, that's helpful. Thanks! I've updated the OP for clarity and exported the job to Upwork here: https://www.upwork.com/jobs/~01c2090be81cc5aedc

[parasharrajat]

Why go through all this fuss #7316 (comment).

Also, this is a small one so I can take it but I have not issues if Manan wants to do it.

Proposal cum suggestion

I would suggest just renaming the index.js to index.web|website.js and index.native.js to index.js for ErrorBodyText. That should work.

🎀 👀 🎀 C+ reviewed

[mananjadhav]

Oh nice. I totally forgot we have support for index.web.js.

Also, this is a small one so I can take it but I have not issues if Manan wants to do it.

fine with whatever you guys decide.

[murataka]

Hello , the thing is the error message tells to user to switch to the web , where the issue won't occur on that type of error on web version

.

However, it is better to know the error type and implement a more detailed error handling mechanism ( Specific cases for specific error types ). As you see above , the errors may still appear on the web version and the user will be confused that the error page recursively tells to switch to the web version.
For a workaround , the error message may be different so that the user won't go to a recursive error situation.
And yes this seems to be a not so difficult issue by the way .

[murataka]

Also this error page should include a submission form as the error will appear on client side and not the server side , so an error submission button would help to identify and fix the bugs fast ...

[roryabraham]

This wasn’t a part of the original requirement if you check the GH issue body

Yes @mananjadhav, that's why I created a separate issue 🙂

@roryabraham can you please confirm the correct url please :)

It's going to be the NewDot URL for whatever environment we're in. So localhost on dev, staging.new.expensify.com on staging, and new.expensify.com on production.

I would suggest just renaming the index.js to index.web|website.js and index.native.js to index.js for ErrorBodyText. That should work.

Good suggestion @parasharrajat 👍

I totally forgot we have support for index.web.js

It has to be index.website.js. You can check out the webpack configs here. Lots of our dependencies use the common index.web.js for the web platform, but since we use Electron, that code is also needed for desktop. So if we want to write web-specific code that will be ignored by our desktop app, we need to use index.website.js.

---

Also @murataka I understand that fixing specific errors for specific crashes is better, but the fact of the matter is that we are not perfect and sometimes bugs make it to production. Of course we want to fix those and give the users an elegant error experience, but if we're landing at this page it means we have no idea what went wrong, and we're displaying this page instead of a blank white screen. It's somewhat unlikely that switching to web from desktop will resolve the bug, but it's possible and an easy step a user can take.

For a workaround , the error message may be different so that the user won't go to a recursive error situation.

If they arrive on the web platform, we won't suggest they switch to another to resolve their issues. So we shouldn't have any "recursive error situation"

Also this error page should include a submission form as the error will appear on client side and not the server side , so an error submission button would help to identify and fix the bugs fast ...

I think it would make sense to try and display their chat with concierge, but if they hit refresh and then land back at this error page, it most likely means there is a bug with this application that makes it so that they won't be able to send or receive chats in-app. In this case, they can chat with concierge via email.

Furthermore, we have Firebase crashlytics which should catch and log these errors, though I'm not sure if it works for web/desktop. cc @Jag96 do you know off-hand?

[roryabraham]

Also, yes it's easy but if I were CME on this I would give it to @mananjadhav since he implemented the generic error page in the first place.

[murataka]

Also, yes it's easy but if I were CME on this I would give it to @mananjadhav since he implemented the generic error page in the first place.

yes, I already am looking at an other issue, as the first step would be to implement some basic error page system which shows different errors, or just change the error messages to not redirect user to web or something similar.

[Jag96]

Furthermore, we have Firebase crashlytics which should catch and log these errors, though I'm not sure if it works for web/desktop. cc @Jag96 do you know off-hand?

Looking in the firebase console, we only have ios/Android errors coming through at the moment. I'm not sure if reporting Desktop/Web is possible w/ firebase, but if it is we haven't done it yet.

[murataka]

@roryabraham , Users may not use the chat to explain such errors, I offer something like
<Errorhandler data={theerrormessage} />
which renders some simple button to send the data using fetch command and not display much information to user. After user clicks that button, display a "thank you , we will take care of it" ... I don't think those kind of errors will happen much though, so I think a simple error feedback mechanism would be just more than enough , and it is not a bad user experience when we feel someone is aware of the issue and it will be fixed soon.

[NikkiWines]

I agree with @roryabraham and I think @mananjadhav is the right fit for this issue. @trjExpensify could you hire them for this role, please?

[MelvinBot]

📣 @mananjadhav You have been assigned to this job by @trjExpensify!
Please apply to this job in Upwork and leave a comment on the Github issue letting us know when we can expect a PR to be ready for review 🧑‍💻
Keep in mind: Code of Conduct | Contributing 📖

[trjExpensify]

Yep, donezo! 👍

[parasharrajat]

Great. Sounds good. See you on the PR @mananjadhav. 😄

[mananjadhav]

It's going to be the NewDot URL for whatever environment we're in. So localhost on dev, staging.new.expensify.com on staging, and new.expensify.com on production.

@roryabraham @parasharrajat @NikkiWines Are we storing the environment-wise URL anywhere?

Few options:

1. CONST.NEW_EXPENSIFY_URL -> Being used right now but it is hardcoded as https://new.expensify.com
2. CONST.NEWDOT -> new.expensify.com this one's hardcoded too
3. CONST.ACTIVE_ENVIRONMENT_NEW_EXPENSIFY_URL -> This either takes staging.new.expensify.com or new.expensify.com based on the environment but not localhost
4. Any other options? or do I need to create a new utility?

[parasharrajat]

You can use CONST.ACTIVE_ENVIRONMENT_NEW_EXPENSIFY_URL. I made that const for the same purpose. I think its fine that it does not use localhost URL for dev but you can change from it .env file.

[roryabraham]

While we're at it, can we please get review all the URL constants and settle on one that's environment-specific and call it NEW_EXPENSIFY_URL, and get rid of all the others?

[parasharrajat]

Yeah, They are confusing and some of them use Cash keyword so probably better to clean this up.

[mananjadhav]

call it NEW_EXPENSIFY_URL, and get rid of all the others?

I can do that, but I'll need some help in wrt to the context of each of the URL constants.

[NikkiWines]

I think we should get rid of expensifyCashURL and URL_EXPENSIFY_CASH AND EXPENSIFY_URL_CASH because why do we have 3 iterations of the same variable with slight variations, all using an outdated name.

Instead, I suggest the following:

1. expensifyURL becomes expensifyURLCom in CONFIG.js (for consistency with URL_EXPENSIFY_COM)
2. expensifyCashURL becomes expensifyURL in CONFIG.js (or expensifyURLNew)
3. URL_EXPENSIFY_CASH becomes EXPENSIFY_URL in CONFIG.js (or EXPENSIFY_URL_NEW)
4. ACTIVE_ENVIRONMENT_NEW_EXPENSIFY_URL becomes ACTIVE_EXPENSIFY_URL or (ACTIVE_EXPENSIFY_URL_NEW) in CONST.js

Optional:

1. Rename ACTIVE_ENVIRONMENT_NEW_EXPENSIFY_URL to EXPENSIFY_URL (or EXPENSIFY_URL_NEW) and replace all instances of the non-active URLs
2. Rename EXPENSIFY_URL_CASH becomes EXPENSIFY_URL in .env

[mananjadhav]

@NikkiWines @roryabraham Is it fine if we track these URL changes as a separate issue? These have impact on unrelated changes and will require testing those instances for 5-6 different places?

[NikkiWines]

That's fine by me 👍

[roryabraham]

Created a follow-up: #7489

[botify]

The solution for this issue has been 🚀 deployed to production 🚀 in version 1.1.34-0 and is now subject to a 7-day regression period 📆. Here is the list of pull requests that resolve this issue:

• fix: Added web link for desktop error page #7364

If no regressions arise, payment will be issued on 2022-02-09. 🎊

[mananjadhav]

@trjExpensify Upwork reminder

[trjExpensify]

Settled up with you @mananjadhav. @parasharrajat sent you an offer for C+ as you hadn't applied for this one yet 👍

[trjExpensify]

Cool, settled up with @parasharrajat now as well. Closing! 👍