fix: fix null-type bugs (#3137)

I ran TypeScript against our code with `strictNullChecks` on. Most of the errors generated are noise, because TypeScript doesn't understand how our `assert` method works. But some were legitimate bugs. They are fixed in this patch.
puppeteer · Aug 24, 2018 · 3d7ae2a · 3d7ae2a
1 parent d1105af
commit 3d7ae2a
Show file tree

Hide file tree

Showing 9 changed files with 16 additions and 12 deletions.
diff --git a/lib/Browser.js b/lib/Browser.js
@@ -161,7 +161,7 @@ class Browser extends EventEmitter {
   }
 
   /**
-   * @param {string} contextId
+   * @param {?string} contextId
    * @return {!Promise<!Puppeteer.Page>}
    */
   async _createPageInContext(contextId) {

diff --git a/lib/Connection.js b/lib/Connection.js
@@ -172,6 +172,7 @@ class CDPSession extends EventEmitter {
     this._lastId = 0;
     /** @type {!Map<number, {resolve: function, reject: function, error: !Error, method: string}>}*/
     this._callbacks = new Map();
+    /** @type {null|Connection|CDPSession} */
     this._connection = connection;
     this._targetType = targetType;
     this._sessionId = sessionId;
@@ -234,6 +235,8 @@ class CDPSession extends EventEmitter {
   }
 
   async detach() {
+    if (!this._connection)
+      throw new Error(`Session already detached. Most likely the ${this._targetType} has been closed.`);
     await this._connection.send('Target.detachFromTarget',  {sessionId: this._sessionId});
   }
 
@@ -266,8 +269,7 @@ function createProtocolError(error, method, object) {
   let message = `Protocol error (${method}): ${object.error.message}`;
   if ('data' in object.error)
     message += ` ${object.error.data}`;
-  if (object.error.message)
-    return rewriteError(error, message);
+  return rewriteError(error, message);
 }
 
 /**

diff --git a/lib/ExecutionContext.js b/lib/ExecutionContext.js
@@ -473,7 +473,7 @@ class ElementHandle extends JSHandle {
 
     const viewport = this._page.viewport();
 
-    if (boundingBox.width > viewport.width || boundingBox.height > viewport.height) {
+    if (viewport && (boundingBox.width > viewport.width || boundingBox.height > viewport.height)) {
       const newViewport = {
         width: Math.max(viewport.width, Math.ceil(boundingBox.width)),
         height: Math.max(viewport.height, Math.ceil(boundingBox.height)),

diff --git a/lib/FrameManager.js b/lib/FrameManager.js
@@ -118,7 +118,6 @@ class FrameManager extends EventEmitter {
   /**
    * @param {string} frameId
    * @param {?string} parentFrameId
-   * @return {?Frame}
    */
   _onFrameAttached(frameId, parentFrameId) {
     if (this._frames.has(frameId))
@@ -265,14 +264,16 @@ class Frame {
     this._parentFrame = parentFrame;
     this._url = '';
     this._id = frameId;
+    this._detached = false;
 
     /** @type {?Promise<!Puppeteer.ElementHandle>} */
     this._documentPromise = null;
-    /** @type {?Promise<!ExecutionContext>} */
-    this._contextPromise = null;
+    /** @type {!Promise<!ExecutionContext>} */
+    this._contextPromise;
     this._contextResolveCallback = null;
     this._setDefaultContext(null);
 
+
     /** @type {!Set<!WaitTask>} */
     this._waitTasks = new Set();
     this._loaderId = '';

diff --git a/lib/Input.js b/lib/Input.js
@@ -38,7 +38,7 @@ class Keyboard {
 
   /**
    * @param {string} key
-   * @param {{text: string}=} options
+   * @param {{text?: string}=} options
    */
   async down(key, options = { text: undefined }) {
     const description = this._keyDescriptionForString(key);

diff --git a/lib/Launcher.js b/lib/Launcher.js
@@ -258,7 +258,7 @@ class Launcher {
   }
 
   /**
-   * @param {!(BrowserOptions & {browserWSEndpoint: string})=} options
+   * @param {!(BrowserOptions & {browserWSEndpoint: string})} options
    * @return {!Promise<!Browser>}
    */
   static async connect(options) {

diff --git a/lib/NetworkManager.js b/lib/NetworkManager.js
@@ -256,7 +256,7 @@ class NetworkManager extends EventEmitter {
     if (!request)
       return;
     const response = new Response(this._client, request, event.response.status, event.response.headers,
-        event.response.fromDiskCache, event.response.fromServiceWorker, event.response.securityDetails);
+        !!event.response.fromDiskCache, !!event.response.fromServiceWorker, event.response.securityDetails);
     request._response = response;
     this.emit(NetworkManager.Events.Response, response);
   }
@@ -353,7 +353,7 @@ class Request {
   }
 
   /**
-   * @return {string}
+   * @return {string|undefined}
    */
   postData() {
     return this._postData;

diff --git a/lib/Page.js b/lib/Page.js
@@ -834,6 +834,7 @@ class Page extends EventEmitter {
       clip.scale = 1;
 
     if (options.fullPage) {
+      assert(this._viewport, 'fullPage screenshots do not work without first setting viewport.');
       const metrics = await this._client.send('Page.getLayoutMetrics');
       const width = Math.ceil(metrics.contentSize.width);
       const height = Math.ceil(metrics.contentSize.height);

diff --git a/lib/Target.js b/lib/Target.js
@@ -77,7 +77,7 @@ class Target {
   }
 
   /**
-   * @return {Puppeteer.Target}
+   * @return {?Puppeteer.Target}
    */
   opener() {
     const { openerId } = this._targetInfo;