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

fix: trying to access undefined process variable #9818

Closed
wants to merge 1 commit into from

Conversation

ccheraa
Copy link

@ccheraa ccheraa commented Aug 2, 2021

What it does

This makes sure that the filesystem package accesses the process variable only when it exists.
See #9815 #9751 for more info

How to test

In our use case, we simply access the browser IDE from a Cypress test that's running on Electron.
The filesystem will throw an exception during its initialization, and the frontend will fail to load.
This PR fixes that.

Review checklist

Reminder for reviewers

Signed-off-by: Charafeddine Cheraa <ccheraa@gmail.com>
Copy link
Member

@vince-fugnitto vince-fugnitto left a comment

Choose a reason for hiding this comment

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

@ccheraa thank you for taking care of the issue 👍

Please be sure to sign the eclipse contributor agreement (eca) with the same email as your authorship in order for us to accept the changes.

@ccheraa
Copy link
Author

ccheraa commented Aug 5, 2021

@vince-fugnitto I am having problems signing up to eclipse.org to be able to sign the eca, I will let you know when it's sorted out.

@@ -29,7 +29,7 @@ import { environment } from '@theia/core/shared/@theia/application-package/lib/e
export const WIN32_MAX_FILE_SIZE_MB = 300; // 300 MB
export const GENERAL_MAX_FILE_SIZE_MB = 16 * 1024; // 16 GB

export const MAX_FILE_SIZE_MB = environment.electron.is() ? process.arch === 'ia32' ? WIN32_MAX_FILE_SIZE_MB : GENERAL_MAX_FILE_SIZE_MB : 32;
export const MAX_FILE_SIZE_MB = environment.electron.is() && 'process' in window ? process.arch === 'ia32' ? WIN32_MAX_FILE_SIZE_MB : GENERAL_MAX_FILE_SIZE_MB : 32;
Copy link
Member

Choose a reason for hiding this comment

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

I think it would be better to have:

export const MAX_FILE_SIZE_MB = typeof process === 'object'
    ? process.arch === 'ia32'
        ? WIN32_MAX_FILE_SIZE_MB
        : GENERAL_MAX_FILE_SIZE_MB
    : 32;

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