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.

Sign up for GitHub

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

Jump to bottom

Unhandled network error rejection when using storage API #5372

Closed
VeryCrazyDog opened this issue Aug 25, 2021 · 6 comments · Fixed by #5578
Closed

Unhandled network error rejection when using storage API #5372

VeryCrazyDog opened this issue Aug 25, 2021 · 6 comments · Fixed by #5578
Labels
api: storage

Comments

@VeryCrazyDog
Copy link

Describe your environment

  • Operating System version: Windows 10
  • Browser version: N/A (Node.js 14)
  • Firebase SDK version: 8.10.0
  • Firebase Product: storage

Describe the problem

Steps to reproduce:

  1. Simulate network error such as disconnect from network, blocking DNS server, blocking outgoing traffic, etc.
  2. Execute storage API such as getMetadata() or getDownloadURL().
  3. Unhandled promise rejection is thrown. A sample console log is at below. My expected result is throwing FirebaseError with code storage/retry-limit-exceeded.
C:\Users\VCD\Developer\playground>node --unhandled-rejections=throw fbstorage-bug.js

C:\Users\VCD\Developer\playground\node_modules\node-fetch\lib\index.js:1461
                        reject(new FetchError(`request to ${request.url} failed, reason: ${err.message}`, 'system', err));
                               ^
FetchError: request to https://firebasestorage.googleapis.com/v0/b/xxxxxxxxxxxxxxxxx.appspot.com/o/image.png failed, reason: getaddrinfo ENOTFOUND firebasestorage.googleapis.com
    at ClientRequest.<anonymous> (C:\Users\VCD\Developer\playground\node_modules\node-fetch\lib\index.js:1461:11)
    at ClientRequest.emit (events.js:315:20)
    at TLSSocket.socketErrorListener (_http_client.js:469:9)
    at TLSSocket.emit (events.js:315:20)
    at emitErrorNT (internal/streams/destroy.js:106:8)
    at emitErrorCloseNT (internal/streams/destroy.js:74:3)
    at processTicksAndRejections (internal/process/task_queues.js:80:21) {
  type: 'system',
  errno: 'ENOTFOUND',
  code: 'ENOTFOUND'
}

Relevant code:

'use strict'

const firebase = require('firebase/app')
require('firebase/storage')

const app = firebase.initializeApp({
  apiKey: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
  authDomain: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
  projectId: 'xxxxxxxxxxxxxxxxx',
  storageBucket: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxx',
  messagingSenderId: 'xxxxxxxxxxxx',
  appId: 'xxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxxx'
})
const storage = app.storage()

// Remember to disconnect the network before running
;(async () => {
  const ref = storage.ref('/image.png')
  const rtv = await ref.getMetadata()
  console.log(rtv)
  console.log('END')
})().catch(error => {
  console.log('EXCEPTION')
  if (error.stack) {
    console.error(error.stack)
  } else {
    console.error(error)
  }
}).finally(async () => {
  await app.delete()
})
@jbalidiong
Copy link
Contributor

Hi @VeryCrazyDog, thanks for reporting this behavior. I tried replicating, but I wasn't able to reproduce the error. I received a lot of ERR_INTERNET_DISCONNECTED before getting the storage/retry-limit-exceeded. I used the same version but adjusted some of the codes. For a guide, here is the snippet that I used:

const ref = storage.ref('/champ.jpg')
const rtv = await ref.getMetadata()
.then( (mdata) =>{
console.log(mdata)
console.log('END')
})
.catch((error) => {
console.log('EXCEPTION')
if (error.stack) {
console.error(error.stack)
} else {
console.error(error)
}
})

@VeryCrazyDog
Copy link
Author

I tried to use the code that you provided on both Windows and Ubuntu, and I can reproduce the problem on both platforms. Are we using the same method to run the code?

mkdir app
cd app
npm install [email protected]
# Put the code to `index.js`, an exact copy of the code in the first post can also reproduce the issue
# Disconnect the machine from network
node index.js

@jbalidiong
Copy link
Contributor

That might be the only difference. If I can replicate the issue, I can have a better look into it. Please share the steps to reproduce the issue and a minimal, but complete sample of a project that I can run locally.

@VeryCrazyDog
Copy link
Author

I have prepared a repository below. Go through the setup and run steps in the README and unhandled rejection should be observed.

https://github.com/VeryCrazyDog/firebase-js-sdk-issue-5372

@jbalidiong
Copy link
Contributor

Thanks for the MCVE, I was able to reproduce the behavior now. Let me check what we can do for this issue or bring someone here that can provide more context about it. I’ll update this thread if I have any information to share.

@hsubox76
Copy link
Contributor

Thanks for the repro, I was able to reproduce it in 9.0.0 compat. Just FYI, going forward, any fixes will be in version 9+ so you'll have to upgrade and change the import paths to require('firebase/compat/app') and require('firebase/compat/storage').

I think we need to catch errors on send here

firebase-js-sdk/packages/storage/src/implementation/request.ts

Line 136 in 1b33fda

.send(self.url_, self.method_, self.body_, self.headers_)
or maybe we should handle the node-fetch error specifically and catch here

firebase-js-sdk/packages/storage/src/platform/node/connection.ts

Line 53 in 1b33fda

return fetch(url, {
.

@Feiyang1 Feiyang1 mentioned this issue Oct 5, 2021
Merged
@google-oss-bot google-oss-bot mentioned this issue Oct 12, 2021
Merged
@firebase firebase locked and limited conversation to collaborators Nov 6, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees
No one assigned
Labels
api: storage
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Do not throw from connection.send in Node.js firebase/firebase-js-sdk
4 participants
@hsubox76 @VeryCrazyDog @google-oss-bot @jbalidiong